diff --git a/crates/agent-gui/test/chat/composer-approval-focus-return.test.mjs b/crates/agent-gui/test/chat/composer-approval-focus-return.test.mjs new file mode 100644 index 000000000..bec7263c5 --- /dev/null +++ b/crates/agent-gui/test/chat/composer-approval-focus-return.test.mjs @@ -0,0 +1,136 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import React0 from "react"; +import { createDomTestEnv } from "../helpers/dom-test-env.mjs"; + +// Like Claude Code: after an approval is answered (the bar that replaced the composer goes +// away) the caret is back in the composer, so the user can type what to do next. + +const svgIcons = new Proxy( + {}, + { + get: (_target, key) => + key === "__esModule" + ? true + : (props) => React0.createElement("svg", { className: props?.className }), + }, +); +let composerProps = null; +const env = await createDomTestEnv({ + mocks: { + "@liveagent/ui/components/IconSet": svgIcons, + "@liveagent/ui/components/chat/fileTypeIcons": svgIcons, + "@liveagent/ui/components/chat/MentionComposer": { + MentionComposer: (props) => { + composerProps = props; + return null; + }, + }, + }, +}); +const { React, act, createRoot } = env; +const doc = env.dom.window.document; +const { ChatComposerBar } = env.loadModule("@liveagent/ui/pages/chat/ChatComposerBar.tsx"); +const { LocaleContext } = env.loadModule("@liveagent/ui/i18n/LocaleContext.tsx"); +const { t: translate } = env.loadModule("@liveagent/app/i18n/config.ts"); +const noop = () => {}; + +let container; +let root; +let modeChanges; +let controlChanges; + +async function render(overrides = {}) { + modeChanges = []; + controlChanges = []; + composerProps = null; + if (!root) { + container = doc.createElement("div"); + doc.body.appendChild(container); + root = createRoot(container); + } + const props = { + surface: "desktop", + conversationId: "c1", + composerRef: { current: null }, + isSending: false, + isUploadingFiles: false, + isInputDisabled: false, + inputPlaceholder: "Ask", + workdir: "/w", + enabledSkills: [], + executionMode: "tools", + hasModels: true, + currentModelLabel: "m", + modelOptions: [], + chatRuntimeControls: { planModeEnabled: false }, + reasoningOptions: [], + thinkingAlwaysOn: false, + commandSafetyMode: "ask", + onCommandSafetyModeChange: (mode) => modeChanges.push(mode), + onSend: noop, + onStop: noop, + onComposerBusyChange: noop, + onSelectModel: noop, + onSelectExecutionMode: noop, + onChatRuntimeControlsChange: (patch) => controlChanges.push(patch), + onPickReadableFiles: noop, + onPickWorkspaceFolder: noop, + onPasteFiles: noop, + pendingUploadedFiles: [], + onRemovePendingUpload: noop, + queuedTurns: [], + queuePaused: false, + onQueuePausedChange: noop, + onRunQueuedTurnNow: noop, + onMoveQueuedTurnUp: noop, + onEditQueuedTurn: noop, + onRemoveQueuedTurn: noop, + ...overrides, + }; + await act(async () => { + root.render( + React.createElement( + LocaleContext.Provider, + { value: { locale: "en-US", t: (key) => translate(key, "en-US") } }, + React.createElement(ChatComposerBar, props), + ), + ); + }); +} + +const focusCalls = []; +const composerRef = { current: { focus: () => focusCalls.push("composer") } }; +const approvalBar = () => + React.createElement("section", { tabIndex: -1, "data-test-approval": "" }, "approval"); + +test("the composer gets the caret back when the answered approval bar goes away", async () => { + focusCalls.length = 0; + await render({ composerRef, approvalBar: approvalBar() }); + const bar = container.querySelector("[data-test-approval]"); + bar.focus(); + assert.equal(doc.activeElement === bar, true); + await render({ composerRef, approvalBar: null }); + assert.deepEqual(focusCalls, ["composer"]); +}); + +test("a caret the user moved elsewhere meanwhile is left alone", async () => { + focusCalls.length = 0; + const other = doc.createElement("input"); + doc.body.appendChild(other); + try { + await render({ composerRef, approvalBar: approvalBar() }); + other.focus(); + await render({ composerRef, approvalBar: null }); + assert.deepEqual(focusCalls, []); + } finally { + other.remove(); + } +}); + +test("rendering without an approval bar never grabs focus", async () => { + focusCalls.length = 0; + await render({ composerRef, approvalBar: null }); + await render({ composerRef, approvalBar: null, inputPlaceholder: "Ask again" }); + assert.deepEqual(focusCalls, []); +}); diff --git a/crates/agent-gui/test/chat/review-approval-cc-focus-r2.test.mjs b/crates/agent-gui/test/chat/review-approval-cc-focus-r2.test.mjs new file mode 100644 index 000000000..96ee84d5e --- /dev/null +++ b/crates/agent-gui/test/chat/review-approval-cc-focus-r2.test.mjs @@ -0,0 +1,330 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { createDomTestEnv } from "../helpers/dom-test-env.mjs"; + +// Second review round of feat-approval-card-cc: the real ChatComposerBar with the REAL +// MentionComposer (so composerIsEmpty / data-composer-draft come from the editor itself, +// including the synchronous clear() every send path calls) and the real ToolApprovalBar in +// the approvalBar slot. Assertions compare strings and booleans only. + +const locale = { locale: "en-US", t: (key) => key }; +const EmptyIcon = () => null; +const icons = new Proxy( + {}, + { + get: (_target, name) => + name === "__esModule" + ? true + : String(name).endsWith("_SVG_MARKUP") + ? "" + : EmptyIcon, + }, +); +const env = await createDomTestEnv({ + mocks: { + "@liveagent/ui/i18n/index": { useLocale: () => locale }, + "@liveagent/ui/components/IconSet": icons, + "@liveagent/ui/components/chat/fileTypeIcons": icons, + }, +}); +window.HTMLElement.prototype.scrollIntoView = () => {}; +window.Range.prototype.getClientRects = () => []; +window.Range.prototype.getBoundingClientRect = () => ({ left: 0, top: 0, right: 0, bottom: 0, width: 0, height: 0 }); +document.execCommand = () => true; + +const { React, act, createRoot } = env; +const { ChatComposerBar } = env.loadModule("@liveagent/ui/pages/chat/ChatComposerBar.tsx"); +const { ToolApprovalBar } = env.loadModule("@liveagent/ui/components/chat/ToolApprovalBar.tsx"); +const noop = () => {}; + +function mountPane(conversationId) { + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const composerRef = { current: null }; + const render = async (approvalBar, extra) => { + const props = { + surface: "desktop", + conversationId, + composerRef, + isSending: true, + isUploadingFiles: false, + isInputDisabled: false, + inputPlaceholder: "Ask", + workdir: "", + enabledSkills: [], + executionMode: "tools", + hasModels: true, + currentModelLabel: "m", + modelOptions: [], + chatRuntimeControls: { planModeEnabled: false }, + reasoningOptions: [], + thinkingAlwaysOn: false, + commandSafetyMode: "ask", + onCommandSafetyModeChange: noop, + onSend: noop, + onStop: noop, + onComposerBusyChange: noop, + onSelectModel: noop, + onSelectExecutionMode: noop, + onChatRuntimeControlsChange: noop, + onPickReadableFiles: noop, + onPickWorkspaceFolder: noop, + onPasteFiles: noop, + pendingUploadedFiles: [], + onRemovePendingUpload: noop, + queuedTurns: [], + queuePaused: false, + onQueuePausedChange: noop, + onRunQueuedTurnNow: noop, + onMoveQueuedTurnUp: noop, + onEditQueuedTurn: noop, + onRemoveQueuedTurn: noop, + approvalBar, + }; + await act(async () => { + extra?.(); + root.render(React.createElement(ChatComposerBar, props)); + }); + }; + const editor = () => container.querySelector('[role="textbox"]'); + const panel = () => container.querySelector("[aria-keyshortcuts='Escape 3']")?.closest("section") ?? null; + const type = async (text) => { + const el = editor(); + await act(async () => { + el.focus(); + el.textContent = text; + const range = document.createRange(); + if (el.firstChild) range.setStart(el.firstChild, text.length); + else range.setStart(el, 0); + range.collapse(true); + const selection = window.getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + el.dispatchEvent(new Event("input", { bubbles: true })); + }); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + }; + const unmount = async () => { + await act(async () => root.unmount()); + container.remove(); + }; + return { render, editor, panel, type, composerRef, container, unmount }; +} + +const decisions = []; +const approvalBar = (pending) => + React.createElement(ToolApprovalBar, { + pending, + onDecide: async (toolCallId, decision, options) => { + decisions.push(options ? `${toolCallId}:${decision}:${options.scope ?? ""}` : `${toolCallId}:${decision}`); + return { ok: true }; + }, + onDecideAll: async () => {}, + }); +const item = (id, rule) => ({ + toolCallId: id, + toolName: "Bash", + summary: rule, + allowSession: true, + sessionScope: rule, + rememberRules: [rule], + defaultScope: "project", +}); + +async function press(key, init = {}) { + const target = document.activeElement; + await act(async () => { + target.dispatchEvent(new KeyboardEvent("keydown", { key, bubbles: true, cancelable: true, ...init })); + await Promise.resolve(); + }); +} + +async function clickInside(el) { + await act(async () => { + el.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, cancelable: true })); + el.focus(); + el.click(); + await Promise.resolve(); + }); +} + +const where = (pane) => + document.activeElement === pane.panel() + ? "panel" + : document.activeElement === pane.editor() + ? "editor" + : document.activeElement === document.body + ? "body" + : (document.activeElement?.tagName ?? "null"); + +// Coordinator's question: a send clears the editor and the first approval of the turn +// arrives right after. composerIsEmpty must be true by then or the card loses its focus +// (Enter/Esc dead right after sending, the common case). Every send path calls +// composer.clear() before the turn starts; clear() reports empty synchronously. +test("right after a send, the first approval card still takes the caret", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + try { + await pane.render(null); + await pane.type("run the tests"); + assert.equal(pane.container.querySelector("[data-composer-draft]") !== null, true, "draft is marked"); + await act(async () => pane.composerRef.current.clear()); + assert.equal(pane.container.querySelector("[data-composer-draft]") === null, true, "mark cleared by clear()"); + await pane.render(approvalBar([item("c1", "Bash(npm test:*)")])); + assert.equal(where(pane), "panel"); + await press("1"); + assert.deepEqual(decisions, ["c1:approve"]); + } finally { + await pane.unmount(); + } +}); + +test("clear() and the approval in the same tick: the card still takes the caret", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + try { + await pane.render(null); + await pane.type("run the tests"); + await pane.render(approvalBar([item("c1", "Bash(npm test:*)")]), () => pane.composerRef.current.clear()); + assert.equal(where(pane), "panel"); + } finally { + await pane.unmount(); + } +}); + +// The P1 fix with the real editor: a draft keeps its caret, keystrokes stay text; a click +// into the card arms the shortcuts; answering gives the caret back with the draft intact. +test("a draft keeps the caret; clicking into the card arms it; the answer returns to the draft", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + try { + await pane.render(null); + await pane.type("use pnpm, see step "); + await pane.render(approvalBar([item("c2", "Bash(rm:*)")])); + assert.equal(where(pane), "editor", "the card did not take the caret from the draft"); + await press("2"); + await press("Enter"); + assert.deepEqual(decisions, [], "keys in the draft do not answer"); + await clickInside(pane.panel()); + await press("3"); + assert.deepEqual(decisions, ["c2:deny"]); + await pane.render(null); + assert.equal(where(pane), "editor", "the caret is back in the composer"); + assert.equal(pane.editor().textContent, "use pnpm, see step ", "draft intact"); + } finally { + await pane.unmount(); + } +}); + +// Queue: answering the first of two keeps the slot mounted; the flag survives the card +// change and the last answer returns the caret. +test("two queued approvals answered by clicking: the caret comes back after the last one", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + try { + await pane.render(null); + await pane.type("x"); + await act(async () => pane.composerRef.current.clear()); + await pane.render(approvalBar([item("c1", "A"), item("c2", "B")])); + await clickInside(pane.container.querySelector("[aria-keyshortcuts='Escape 3']")); + await pane.render(approvalBar([item("c2", "B")])); + assert.equal(where(pane), "panel", "the second card has the caret"); + await press("1"); + await pane.render(null); + assert.deepEqual(decisions, ["c1:deny", "c2:approve"]); + assert.equal(where(pane), "editor"); + } finally { + await pane.unmount(); + } +}); + +// Tab / Shift+Tab out of the card to another control, then the approval goes away (remote +// answer): the caret stays where the user put it, even after that control is removed. +test("leaving the card with Tab clears the flag, even if the next control later goes away", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + const outside = document.body.appendChild(document.createElement("button")); + try { + await pane.render(null); + await pane.render(approvalBar([item("c1", "A")])); + assert.equal(where(pane), "panel"); + await act(async () => outside.focus()); + outside.remove(); + assert.equal(document.activeElement === document.body, true); + await pane.render(null); + assert.equal(where(pane), "body"); + } finally { + outside.remove(); + await pane.unmount(); + } +}); + +// The ▾ menu (real base-ui Menu, portaled to , outside the slot): "deny all" picked +// with the mouse answers every approval; the caret should come back like any other answer. +test("deny all from the ▾ menu gives the caret back to the composer", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + let decidedAll = ""; + const bar = (pending) => + React.createElement(ToolApprovalBar, { + pending, + onDecide: async () => ({ ok: true }), + onDecideAll: async (decision) => { + decidedAll = decision; + }, + }); + try { + await pane.render(null); + await pane.render(bar([item("c1", "A"), item("c2", "B")])); + assert.equal(where(pane), "panel"); + const trigger = pane.container.querySelector("[aria-label='chat.toolApproval.moreActions']"); + assert.equal(Boolean(trigger), true, "the ▾ trigger is rendered"); + await act(async () => { + trigger.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, cancelable: true, button: 0, pointerType: "mouse" })); + trigger.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true, button: 0 })); + trigger.dispatchEvent(new PointerEvent("pointerup", { bubbles: true, cancelable: true, button: 0, pointerType: "mouse" })); + trigger.dispatchEvent(new MouseEvent("mouseup", { bubbles: true, cancelable: true, button: 0 })); + trigger.click(); + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + const denyAll = [...document.querySelectorAll('[role="menuitem"]')].find( + (el) => el.textContent === "chat.toolApproval.denyAll", + ); + assert.equal(Boolean(denyAll), true, "the menu opened with deny all"); + assert.equal(pane.container.contains(denyAll), false, "the menu is portaled outside the slot"); + await act(async () => { + denyAll.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, cancelable: true, button: 0, pointerType: "mouse" })); + denyAll.dispatchEvent(new MouseEvent("mousedown", { bubbles: true, cancelable: true, button: 0 })); + denyAll.dispatchEvent(new PointerEvent("pointerup", { bubbles: true, cancelable: true, button: 0, pointerType: "mouse" })); + denyAll.dispatchEvent(new MouseEvent("mouseup", { bubbles: true, cancelable: true, button: 0 })); + denyAll.click(); + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + assert.equal(decidedAll, "deny"); + await pane.render(null); + assert.equal(where(pane), "editor"); + } finally { + await pane.unmount(); + } +}); + +// A click on non-focusable text elsewhere (focus falls to body without any blur target). +test("a click on plain text outside the card clears the flag", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + const text = document.body.appendChild(document.createElement("p")); + try { + await pane.render(null); + await pane.render(approvalBar([item("c1", "A")])); + await act(async () => { + text.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true, cancelable: true })); + document.activeElement?.blur(); + }); + await pane.render(null); + assert.equal(where(pane), "body"); + } finally { + text.remove(); + await pane.unmount(); + } +}); diff --git a/crates/agent-gui/test/chat/review-approval-cc-focus.test.mjs b/crates/agent-gui/test/chat/review-approval-cc-focus.test.mjs new file mode 100644 index 000000000..555668904 --- /dev/null +++ b/crates/agent-gui/test/chat/review-approval-cc-focus.test.mjs @@ -0,0 +1,248 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import React0 from "react"; +import { createDomTestEnv } from "../helpers/dom-test-env.mjs"; + +// Review of the Claude-Code-style approval card (feat-approval-card-cc): the real +// ChatComposerBar hosting the real ToolApprovalBar in its approvalBar slot, the way +// ConversationPaneHost (GUI) and useGatewayChatPresentation / GatewayConversationPaneHost +// (WebUI) wire it. MentionComposer is replaced by a plain textarea so the caret is real. +// Assertions compare strings and booleans only. + +const svgIcons = new Proxy( + {}, + { + get: (_target, key) => + key === "__esModule" + ? true + : (props) => React0.createElement("svg", { className: props?.className }), + }, +); +const composerPropsById = new Map(); +const env = await createDomTestEnv({ + mocks: { + "@liveagent/ui/components/IconSet": svgIcons, + "@liveagent/ui/components/chat/fileTypeIcons": svgIcons, + "@liveagent/ui/components/chat/MentionComposer": { + MentionComposer: (props) => { + composerPropsById.set(props.currentConversationId, props); + return React0.createElement("textarea", { + "data-test-composer": props.currentConversationId, + ref: (el) => { + if (!props.ref) return; + props.ref.current = el + ? { + focus: () => el.focus(), + getDraft: () => ({ segments: [{ type: "text", text: el.value }] }), + } + : null; + }, + }); + }, + }, + }, +}); +const { React, act, createRoot } = env; +const doc = env.dom.window.document; +const { ChatComposerBar } = env.loadModule("@liveagent/ui/pages/chat/ChatComposerBar.tsx"); +const { ToolApprovalBar } = env.loadModule("@liveagent/ui/components/chat/ToolApprovalBar.tsx"); +const { LocaleContext } = env.loadModule("@liveagent/ui/i18n/LocaleContext.tsx"); +const { t: translate } = env.loadModule("@liveagent/app/i18n/config.ts"); +const noop = () => {}; + +function mountPane(conversationId) { + const container = doc.createElement("div"); + doc.body.appendChild(container); + const root = createRoot(container); + const composerRef = { current: null }; + const render = async (approvalBar) => { + const props = { + surface: "desktop", + conversationId, + composerRef, + isSending: true, + isUploadingFiles: false, + isInputDisabled: false, + inputPlaceholder: "Ask", + workdir: "/w", + enabledSkills: [], + executionMode: "tools", + hasModels: true, + currentModelLabel: "m", + modelOptions: [], + chatRuntimeControls: { planModeEnabled: false }, + reasoningOptions: [], + thinkingAlwaysOn: false, + commandSafetyMode: "ask", + onCommandSafetyModeChange: noop, + onSend: noop, + onStop: noop, + onComposerBusyChange: noop, + onSelectModel: noop, + onSelectExecutionMode: noop, + onChatRuntimeControlsChange: noop, + onPickReadableFiles: noop, + onPickWorkspaceFolder: noop, + onPasteFiles: noop, + pendingUploadedFiles: [], + onRemovePendingUpload: noop, + queuedTurns: [], + queuePaused: false, + onQueuePausedChange: noop, + onRunQueuedTurnNow: noop, + onMoveQueuedTurnUp: noop, + onEditQueuedTurn: noop, + onRemoveQueuedTurn: noop, + approvalBar, + }; + await act(async () => { + root.render( + React.createElement( + LocaleContext.Provider, + { value: { locale: "en-US", t: (key) => translate(key, "en-US") } }, + React.createElement(ChatComposerBar, props), + ), + ); + }); + }; + const textarea = () => container.querySelector(`[data-test-composer="${conversationId}"]`); + const panel = () => container.querySelector("[aria-keyshortcuts='Escape 3']")?.closest("section") ?? null; + const unmount = async () => { + await act(async () => root.unmount()); + container.remove(); + }; + return { render, textarea, panel, unmount }; +} + +const decisions = []; +const approvalBar = (pending) => + React.createElement(ToolApprovalBar, { + pending, + onDecide: async (toolCallId, decision, options) => { + decisions.push(options ? `${toolCallId}:${decision}:${options.scope ?? ""}` : `${toolCallId}:${decision}`); + return { ok: true }; + }, + onDecideAll: async () => {}, + }); + +const npmTest = { + toolCallId: "c1", + toolName: "Bash", + summary: "npm test", + allowSession: true, + sessionScope: "Bash(npm test:*)", + rememberRules: ["Bash(npm test:*)"], + defaultScope: "project", +}; +// What the model tries right after being told "no": another command, another approval. +const rmRf = { + toolCallId: "c2", + toolName: "Bash", + summary: "rm -rf build && npm run build", + allowSession: true, + sessionScope: "Bash(rm:*)", + rememberRules: ["Bash(rm:*)"], + defaultScope: "project", +}; + +async function press(key) { + const target = doc.activeElement; + await act(async () => { + target.dispatchEvent(new env.dom.window.KeyboardEvent("keydown", { key, bubbles: true, cancelable: true })); + await Promise.resolve(); + }); +} + +async function typeIntoComposer(pane, conversationId, text) { + const el = pane.textarea(); + await act(async () => { + el.value = text; + el.dispatchEvent(new env.dom.window.Event("input", { bubbles: true })); + composerPropsById.get(conversationId)?.onEmptyChange?.(text.length === 0); + }); +} + +// Design doc 2026-10-31-approval-experience.md §2.6: "按键只在卡片有焦点或输入框为空时生效， +// 不抢正在输入的文字" and the test list: "输入框有文字时数字键不触发". The deny now hands the +// caret to the composer so the user writes what to do next; when the turn goes on and the +// model's next tool call needs approval mid-sentence, the card takes the caret from the +// half-written draft and the user's next keystrokes answer it. +for (const [key, expected] of [ + ["2", "c2:approve_session:project"], + ["Enter", "c2:approve"], +]) { + test(`after a deny, ${key} typed into the composer draft does not answer the next approval (${expected})`, async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + try { + await pane.render(approvalBar([npmTest])); + assert.equal(doc.activeElement === pane.panel(), true, "the first card has the caret"); + await press("3"); + assert.deepEqual(decisions, ["c1:deny"]); + await pane.render(null); + assert.equal(doc.activeElement === pane.textarea(), true, "the composer has the caret back"); + + await typeIntoComposer(pane, "conv-a", "use pnpm instead, see step "); + // The model's next attempt arrives while the user is still writing. + await pane.render(approvalBar([rmRf])); + await press(key); + + assert.deepEqual( + decisions, + ["c1:deny"], + `a ${key} meant for the draft answered the next approval`, + ); + } finally { + await pane.unmount(); + } + }); +} + +// Risk 1 (verification, expected to pass): with two approvals queued, answering the first +// keeps the bar mounted, so the composer must not take the caret and the next card keeps +// the shortcuts. +test("answering the first of two queued approvals keeps the caret on the card", async () => { + decisions.length = 0; + const pane = mountPane("conv-a"); + try { + await pane.render(approvalBar([npmTest, rmRf])); + await press("3"); + await pane.render(approvalBar([rmRf])); + assert.equal(doc.activeElement === pane.panel(), true, "the second card has the caret"); + assert.equal(doc.activeElement === pane.textarea(), false); + await press("1"); + assert.deepEqual(decisions, ["c1:deny", "c2:approve"]); + } finally { + await pane.unmount(); + } +}); + +// Two panes (GUI workbench / WebUI GatewayConversationPaneHost). The user works in pane B +// and clicks on transcript text there (focus falls to ). Pane A's approval then goes +// away without the user touching it (answered from the other client of a mirrored session, +// run stopped, timeout). Pane A's composer takes the caret although its approval bar never +// held it at that point, so the user's next keystrokes land in a pane they are not using. +test("an approval that goes away while the user is elsewhere does not pull the caret into its pane", async () => { + decisions.length = 0; + const paneA = mountPane("conv-a"); + const paneB = mountPane("conv-b"); + try { + await paneA.render(approvalBar([npmTest])); + await paneB.render(null); + // The user moves to pane B, types, then clicks on its transcript text. + paneB.textarea().focus(); + paneB.textarea().blur(); + assert.equal(doc.activeElement === doc.body, true, "focus is on nothing in pane B"); + + await paneA.render(null); + + assert.equal( + doc.activeElement?.getAttribute?.("data-test-composer") ?? "body", + "body", + "pane A's composer took the caret", + ); + } finally { + await paneA.unmount(); + await paneB.unmount(); + } +}); diff --git a/crates/agent-gui/test/chat/review-approval-cc-keys.test.mjs b/crates/agent-gui/test/chat/review-approval-cc-keys.test.mjs new file mode 100644 index 000000000..db2babb74 --- /dev/null +++ b/crates/agent-gui/test/chat/review-approval-cc-keys.test.mjs @@ -0,0 +1,104 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { createDomTestEnv } from "../helpers/dom-test-env.mjs"; + +// Review of feat-approval-card-cc (verification, expected to pass): shortcut guards and the +// remember-scope label on the real ToolApprovalBar. Strings and booleans only in asserts. + +async function renderBar(pending, { holdDecisions = false } = {}) { + const decisions = []; + const env = await createDomTestEnv({ mocks: { + "@liveagent/ui/components/IconSet": new Proxy({}, { get: () => () => null }), + "@liveagent/ui/components/ui/button": { Button: ({ children, size, variant, ...props }) => env.React.createElement("button", props, children) }, + "@liveagent/ui/components/ui/dropdown-menu": { + DropdownMenu: ({ children }) => env.React.createElement("div", null, children), + DropdownMenuTrigger: ({ render, children, ...props }) => env.React.cloneElement(render, props, children), + DropdownMenuContent: ({ children }) => env.React.createElement("div", { role: "menu" }, children), + DropdownMenuItem: ({ children, onSelect, disabled }) => env.React.createElement("button", { role: "menuitem", disabled, onClick: onSelect }, children), + }, + } }); + const { ToolApprovalBar } = env.loadModule("@liveagent/ui/components/chat/ToolApprovalBar.tsx"); + const host = document.body.appendChild(document.createElement("div")); + const root = env.createRoot(host); + const onDecide = (toolCallId, decision, options) => { + decisions.push(options ? `${toolCallId}:${decision}:${options.scope ?? ""}` : `${toolCallId}:${decision}`); + return holdDecisions ? new Promise(() => {}) : Promise.resolve({ ok: true }); + }; + await env.act(async () => root.render(env.React.createElement(ToolApprovalBar, { pending, onDecide, onDecideAll: async () => {} }))); + const panel = host.querySelector("section"); + const key = async (name, init = {}, target = panel) => { + await env.act(async () => { + target.dispatchEvent(new KeyboardEvent("keydown", { key: name, bubbles: true, cancelable: true, ...init })); + await Promise.resolve(); + }); + }; + const cleanup = async () => { + await env.act(async () => root.unmount()); + host.remove(); + env.cleanup(); + }; + return { host, panel, decisions, key, cleanup }; +} + +const base = { toolCallId: "c1", toolName: "Bash", summary: "npm test" }; + +test("2 does nothing without rules and without a session grant", async () => { + const { decisions, key, cleanup } = await renderBar([base]); + try { + await key("2"); + assert.deepEqual(decisions, []); + } finally { + await cleanup(); + } +}); + +test("2 without rules but with a session grant is the old session approval, no scope", async () => { + const { host, decisions, key, cleanup } = await renderBar([{ ...base, allowSession: true, sessionWide: true }]); + try { + assert.equal(host.querySelector("[data-remember-scope]") === null, true); + await key("2"); + assert.deepEqual(decisions, ["c1:approve_session"]); + } finally { + await cleanup(); + } +}); + +test("rules without defaultScope remember for this session and say so", async () => { + const { host, decisions, key, cleanup } = await renderBar([{ ...base, allowSession: true, rememberRules: ["Bash(npm test:*)"] }]); + try { + assert.equal(host.querySelector("[data-remember-scope]")?.textContent ?? "", "本会话"); + assert.equal(host.querySelector("[data-remember-scope]")?.closest(".truncate") ?? null, null); + await key("2"); + assert.deepEqual(decisions, ["c1:approve_session:session"]); + } finally { + await cleanup(); + } +}); + +test("rules are not offered when the backend does not allow remembering", async () => { + const { host, decisions, key, cleanup } = await renderBar([{ ...base, rememberRules: ["Bash(npm test:*)"] }]); + try { + assert.equal(host.querySelector("[data-remember-scope]") === null, true); + await key("2"); + assert.deepEqual(decisions, []); + } finally { + await cleanup(); + } +}); + +test("3 with a modifier, while submitting, or on a button does not deny", async () => { + const { host, decisions, key, cleanup } = await renderBar([{ ...base, allowSession: true, rememberRules: ["R"] }], { holdDecisions: true }); + try { + for (const mod of ["ctrlKey", "altKey", "metaKey", "shiftKey"]) await key("3", { [mod]: true }); + assert.deepEqual(decisions, []); + const button = host.querySelector("[aria-keyshortcuts='2']"); + await key("3", {}, button); + assert.deepEqual(decisions, [], "digits on a button are the button's own"); + await key("1"); + await key("3"); + await key("Escape"); + assert.deepEqual(decisions, ["c1:approve"], "nothing else while the first answer is in flight"); + } finally { + await cleanup(); + } +}); diff --git a/crates/agent-gui/test/chat/review-la2b-approval-card.test.mjs b/crates/agent-gui/test/chat/review-la2b-approval-card.test.mjs index d7833f4bf..8c6c615f9 100644 --- a/crates/agent-gui/test/chat/review-la2b-approval-card.test.mjs +++ b/crates/agent-gui/test/chat/review-la2b-approval-card.test.mjs @@ -124,63 +124,3 @@ test("the real approval card renders a bounded diff for a large desktop Write", // inputs (MentionComposerInputUtils.isImeKeyboardEvent, ConversationSearchDialog, // ChatHistorySidebar) treat keyCode 229 as an IME key. // --------------------------------------------------------------------------------- - -test("committing an IME candidate with Enter in the reason field does not deny (WebKit order)", async () => { - const env = await createDomTestEnv({ - mocks: { - "@liveagent/ui/components/IconSet": new Proxy({}, { get: () => () => null }), - "@liveagent/ui/components/ui/button": { - Button: ({ children, size, variant, ...props }) => env.React.createElement("button", props, children), - }, - "@liveagent/ui/components/ui/dropdown-menu": { - DropdownMenu: ({ children }) => env.React.createElement("div", null, children), - DropdownMenuTrigger: ({ render, children, ...props }) => env.React.cloneElement(render, props, children), - DropdownMenuContent: ({ children }) => env.React.createElement("div", { role: "menu" }, children), - DropdownMenuItem: ({ children, onSelect, disabled }) => - env.React.createElement("button", { role: "menuitem", disabled, onClick: onSelect }, children), - }, - "@liveagent/ui/components/chat/EditDiffView": { EditDiffView: () => null }, - }, - }); - const { ToolApprovalBar } = env.loadModule("@liveagent/ui/components/chat/ToolApprovalBar.tsx"); - const decisions = []; - const host = document.body.appendChild(document.createElement("div")); - const root = env.createRoot(host); - try { - await env.act(async () => - root.render( - env.React.createElement(ToolApprovalBar, { - pending: [{ toolCallId: "c1", toolName: "Bash", summary: "rm -rf build" }], - onDecide: async (id, decision, options) => { - decisions.push(JSON.stringify([id, decision, options ?? null])); - return { ok: true }; - }, - onDecideAll: async () => {}, - }), - ), - ); - const panel = host.querySelector("section"); - await env.act(async () => { - panel.dispatchEvent(new KeyboardEvent("keydown", { key: "3", bubbles: true, cancelable: true })); - }); - const input = host.querySelector("input[data-deny-reason]"); - assert.equal(Boolean(input), true, "the reason field opens"); - await env.act(async () => { - input.dispatchEvent(new window.CompositionEvent("compositionstart", { bubbles: true })); - const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, "value").set; - setter.call(input, "改用"); - input.dispatchEvent(new Event("input", { bubbles: true })); - input.dispatchEvent(new window.CompositionEvent("compositionend", { bubbles: true, data: "改用" })); - const enter = new KeyboardEvent("keydown", { key: "Enter", bubbles: true, cancelable: true }); - Object.defineProperty(enter, "keyCode", { value: 229 }); - Object.defineProperty(enter, "which", { value: 229 }); - input.dispatchEvent(enter); - await Promise.resolve(); - }); - assert.deepEqual(decisions, [], `the IME commit Enter submitted a denial: ${decisions.join(" ")}`); - } finally { - await env.act(async () => root.unmount()); - host.remove(); - env.cleanup(); - } -}); diff --git a/crates/agent-gui/test/chat/tool-approval-card.test.mjs b/crates/agent-gui/test/chat/tool-approval-card.test.mjs index b5536c785..0646d566b 100644 --- a/crates/agent-gui/test/chat/tool-approval-card.test.mjs +++ b/crates/agent-gui/test/chat/tool-approval-card.test.mjs @@ -73,55 +73,48 @@ test("1 allows once and 2 remembers in the default scope", async () => { } }); -test("the remember option names the rules and offers the three scopes", async () => { - const { env, host, decisions, key, cleanup } = await renderBar([remembering]); +// Like Claude Code: 2 remembers in the scope the backend suggests (no scope picker; Settings +// → 权限 changes or deletes a rule), and there is one deny (Esc or 3) with no reason field — +// the user tells the model what to do next in the composer. +test("the remember option names the rules and the scope they go to, without a picker", async () => { + const { host, decisions, key, cleanup } = await renderBar([remembering]); try { assert.match(host.textContent, /Bash\(npm test:\*\)/); - const select = host.querySelector("select[data-remember-scope]"); - assert.equal(Boolean(select), true, "a scope select is shown"); - assert.deepEqual([...select.options].map((o) => o.value), ["session", "project", "user"]); - assert.equal(select.value, "project"); - await env.act(async () => { - select.value = "user"; - select.dispatchEvent(new Event("change", { bubbles: true })); - }); + assert.match(host.textContent, /本项目/); + assert.equal(Boolean(host.querySelector("select")), false, "no scope picker"); await key("2"); - assert.deepEqual(decisions, [["c1", "approve_session", { scope: "user" }]]); + assert.deepEqual(decisions, [["c1", "approve_session", { scope: "project" }]]); } finally { await cleanup(); } }); -test("3 opens a reason field; Enter there denies with the reason and digits there are text", async () => { - const { env, host, decisions, key, cleanup } = await renderBar([remembering]); +test("the backend's default scope decides where 2 remembers", async () => { + const { host, decisions, key, cleanup } = await renderBar([{ ...remembering, defaultScope: "user" }]); try { - await key("3"); - const input = host.querySelector("input[data-deny-reason]"); - assert.equal(Boolean(input), true, "the reason field opens"); - assert.equal(document.activeElement === input, true, "and takes focus"); - await key("1", input); - await key("2", input); - assert.deepEqual(decisions, [], "digits typed in the field do not answer"); - await env.act(async () => { - const setter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, "value").set; - setter.call(input, "edit utils.ts instead"); - input.dispatchEvent(new Event("input", { bubbles: true })); - }); - await key("Enter", input); - assert.deepEqual(decisions, [["c1", "deny", { reason: "edit utils.ts instead" }]]); + assert.match(host.textContent, /所有项目/); + await key("2"); + assert.deepEqual(decisions, [["c1", "approve_session", { scope: "user" }]]); } finally { await cleanup(); } }); -test("Esc in the reason field closes it; Esc on the card still denies", async () => { +test("3 denies like Esc, and there is no reason field", async () => { const { host, decisions, key, cleanup } = await renderBar([remembering]); try { + assert.equal(Boolean(host.querySelector("input")), false, "no reason field"); + assert.equal(/拒绝并说明/.test(host.textContent), false, "one deny only"); await key("3"); - const input = host.querySelector("input[data-deny-reason]"); - await key("Escape", input); - assert.equal(Boolean(host.querySelector("input[data-deny-reason]")), false, "the field closes"); - assert.deepEqual(decisions, [], "closing the field does not deny"); + assert.deepEqual(decisions, [["c1", "deny"]]); + } finally { + await cleanup(); + } +}); + +test("Esc on the card denies", async () => { + const { decisions, key, cleanup } = await renderBar([remembering]); + try { await key("Escape"); assert.deepEqual(decisions, [["c1", "deny"]]); } finally { diff --git a/crates/agent-ui/src/components/chat/ToolApprovalBar.tsx b/crates/agent-ui/src/components/chat/ToolApprovalBar.tsx index d5803eda1..8c2c35421 100644 --- a/crates/agent-ui/src/components/chat/ToolApprovalBar.tsx +++ b/crates/agent-ui/src/components/chat/ToolApprovalBar.tsx @@ -5,7 +5,6 @@ // WebUI 走网关 tool_approval，端差异一律留在各端宿主。 import { EditDiffView } from "@liveagent/ui/components/chat/EditDiffView"; -import { isImeKeyboardEvent } from "@liveagent/ui/components/chat/MentionComposerInputUtils"; import { ChevronDown, Loader2, Terminal } from "@liveagent/ui/components/IconSet"; import { Button } from "@liveagent/ui/components/ui/button"; import { @@ -24,8 +23,6 @@ import type { ToolApprovalRememberScope, } from "../../lib/chat/toolApprovalArgs"; -const REMEMBER_SCOPES: readonly ToolApprovalRememberScope[] = ["session", "project", "user"]; - /** approve:本次放行;deny:本次拒绝;approve_session:请后端在本会话内记住这条授权范围。 */ export type ToolApprovalDecision = "approve" | "deny" | "approve_session"; @@ -44,7 +41,7 @@ export type PendingApprovalItem = { sessionScope?: string; /** 本次允许即对本会话同类操作生效(如 Computer Use);正文与按钮据此说明。 */ sessionWide?: boolean; - /** 允许并记住会存的规则;非空时可以选作用域。 */ + /** 允许并记住会存的规则;非空时按 2 记到 defaultScope(和 Claude Code 一样不在卡片上选作用域)。 */ rememberRules?: string[]; defaultScope?: ToolApprovalRememberScope; /** Edit/Write 的改动预览。 */ @@ -110,32 +107,13 @@ export function ToolApprovalBar({ text: string; } | null>(null); const [moreOpen, setMoreOpen] = useState(false); - const [reasonOpen, setReasonOpen] = useState(false); - const [reason, setReason] = useState(""); const panelRef = useRef(null); - const reasonRef = useRef(null); const deadlines = pending .map((item) => item.deadlineAt) .filter((value): value is number => typeof value === "number"); const remainingMs = useEarliestCountdown(pending.length > 0, deadlines); const submitting = submittingAction !== null; const currentToolCallId = pending[0]?.toolCallId ?? ""; - const currentDefaultScope = pending[0]?.defaultScope ?? "session"; - const [rememberScope, setRememberScope] = useState(currentDefaultScope); - const [cardFor, setCardFor] = useState(currentToolCallId); - - // Each approval starts from its own default scope with the reason field closed, - // reset while rendering so the next card never shows the previous card's state. - if (cardFor !== currentToolCallId) { - setCardFor(currentToolCallId); - setRememberScope(currentDefaultScope); - setReasonOpen(false); - setReason(""); - } - - useEffect(() => { - if (reasonOpen) reasonRef.current?.focus({ preventScroll: true }); - }, [reasonOpen]); useEffect(() => { if (!currentToolCallId) return; @@ -153,6 +131,10 @@ export function ToolApprovalBar({ // rendered; otherwise Enter/Escape would silently stop working right after // sending a message, which is the common case. const active = document.activeElement as HTMLElement | null; + // Someone typing in the composer (say, what to do after a deny) keeps their caret: an + // approval that lands mid-sentence must not turn the next "2" or Enter into an answer. + // The shortcuts work once they click or Tab into the bar. + if (active?.closest("[data-composer-draft]")) return; const activeIsEditable = active !== null && (active.tagName === "INPUT" || active.tagName === "TEXTAREA" || active.isContentEditable); @@ -175,6 +157,7 @@ export function ToolApprovalBar({ const offerRemember = rememberRules.length > 0; const offerLegacySession = offerSession && !offerRemember; const hasMoreActions = offerLegacySession || pending.length > 1; + const rememberScope = current.defaultScope ?? "session"; const sessionTarget = current.sessionScope ? `${displayToolName} · ${current.sessionScope}` : displayToolName; @@ -218,14 +201,10 @@ export function ToolApprovalBar({ ); } }; - const deny = (withReason?: string) => { - const text = withReason?.trim(); - void runGuarded(`deny:${current.toolCallId}`, () => - text - ? onDecide(current.toolCallId, "deny", { reason: text }) - : onDecide(current.toolCallId, "deny"), - ); - }; + // One deny, like Claude Code's "No, and tell Claude what to do differently": the user + // says what to do next in the composer, which gets the caret back once the bar is gone. + const deny = () => + void runGuarded(`deny:${current.toolCallId}`, () => onDecide(current.toolCallId, "deny")); return (
void> = { Enter: approveOnce, "1": approveOnce, "2": remember, - "3": () => { - setReasonOpen(true); - reasonRef.current?.focus({ preventScroll: true }); - }, + "3": deny, }; const handle = handlers[event.key]; if (!handle || (event.key === "2" && !offerRemember && !offerLegacySession)) return; @@ -373,26 +349,11 @@ export function ToolApprovalBar({
{t("chat.toolApproval.rememberRules").replace("{rules}", rememberRules.join(", "))} + {" · "} + + {t(`chat.toolApproval.scope.${rememberScope}`)} + -
) : null} - - {reasonOpen ? ( -
- setReason(event.target.value)} - onKeyDown={(event) => { - // WebKit sends the Enter that commits an IME candidate after - // compositionend (keyCode 229); that Enter picks a word, it does not deny. - if (isImeKeyboardEvent(event)) return; - if (event.key === "Enter") { - event.preventDefault(); - event.stopPropagation(); - deny(reason); - } else if (event.key === "Escape") { - event.preventDefault(); - event.stopPropagation(); - setReasonOpen(false); - setReason(""); - panelRef.current?.focus({ preventScroll: true }); - } - }} - className="h-8 min-w-0 flex-1 rounded-md border border-border bg-background px-2 text-xs text-foreground outline-none focus-visible:ring-2 focus-visible:ring-primary/35" - /> -
- ) : null}
@@ -450,27 +380,13 @@ export function ToolApprovalBar({ ) : null} - {reasonOpen ? null : ( - - )} -
) : null} - {approvalBar} + {/* Holds the approval bar so the effect above can tell whether the caret is on it. */} +
+ {approvalBar} +
{/* 澄清面板浮在输入卡片正上方（原先是卡片内嵌段）：与队列面板同级， 直接顶在卡片上缘。审批面板接管输入区时同样让位。 */} {clarifyOpen && runClarifyTurn && approvalBar == null ? ( @@ -1496,6 +1545,7 @@ export const ChatComposerBar = memo(function ChatComposerBar(props: ChatComposer {/* biome-ignore lint/a11y/noStaticElementInteractions: Escape 捕获仅在展开态生效，焦点始终在内部 textbox 上，包装层不参与 Tab 序。 */}