From 45a5ba8637a60fc1bc6abe7372997d4e32a13871 Mon Sep 17 00:00:00 2001 From: Edvard Chen Date: Thu, 16 Apr 2026 10:23:05 +0800 Subject: [PATCH] Fix duplicate command args for generic ACP providers (#444) * Fix duplicate command args for generic ACP providers * Fix desktop IME enter handling --- packages/app/src/components/message-input.tsx | 3 +- .../app/src/hooks/use-keyboard-shortcuts.ts | 7 +++ packages/app/src/utils/keyboard-ime.test.ts | 31 +++++++++++++ packages/app/src/utils/keyboard-ime.ts | 5 +++ .../agent/providers/generic-acp-agent.test.ts | 45 +++++++++++++++++++ .../agent/providers/generic-acp-agent.ts | 4 -- 6 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 packages/app/src/utils/keyboard-ime.test.ts create mode 100644 packages/app/src/utils/keyboard-ime.ts create mode 100644 packages/server/src/server/agent/providers/generic-acp-agent.test.ts diff --git a/packages/app/src/components/message-input.tsx b/packages/app/src/components/message-input.tsx index 44ce35c52..3965435e0 100644 --- a/packages/app/src/components/message-input.tsx +++ b/packages/app/src/components/message-input.tsx @@ -45,6 +45,7 @@ import { useShortcutKeys } from "@/hooks/use-shortcut-keys"; import { formatShortcut } from "@/utils/format-shortcut"; import { getShortcutOs } from "@/utils/shortcut-platform"; import type { MessageInputKeyboardActionKind } from "@/keyboard/actions"; +import { isImeComposingKeyboardEvent } from "@/utils/keyboard-ime"; import { markScrollInvestigationEvent, markScrollInvestigationRender, @@ -895,7 +896,7 @@ export const MessageInput = forwardRef(funct // IME composition in progress (e.g. CJK input) — all key events belong to the // IME, not the app. keyCode 229 is a Chromium fallback for when isComposing is // cleared before the keydown fires. - if (event.nativeEvent.isComposing || event.nativeEvent.keyCode === 229) return; + if (isImeComposingKeyboardEvent(event.nativeEvent)) return; // Allow parent to intercept key events (e.g., for autocomplete navigation) if (onKeyPressCallback) { diff --git a/packages/app/src/hooks/use-keyboard-shortcuts.ts b/packages/app/src/hooks/use-keyboard-shortcuts.ts index 9031d2b0c..84625295b 100644 --- a/packages/app/src/hooks/use-keyboard-shortcuts.ts +++ b/packages/app/src/hooks/use-keyboard-shortcuts.ts @@ -27,6 +27,7 @@ import { getShortcutOs } from "@/utils/shortcut-platform"; import { useOpenProjectPicker } from "@/hooks/use-open-project-picker"; import { useKeyboardShortcutOverrides } from "@/hooks/use-keyboard-shortcut-overrides"; import { isNative } from "@/constants/platform"; +import { isImeComposingKeyboardEvent } from "@/utils/keyboard-ime"; export function useKeyboardShortcuts({ enabled, @@ -323,6 +324,12 @@ export function useKeyboardShortcuts({ return; } + // During IME composition, Enter confirms the candidate selection and must + // not route through global shortcuts like message send. + if (isImeComposingKeyboardEvent(event)) { + return; + } + const store = useKeyboardShortcutsStore.getState(); if (store.capturingShortcut) { return; diff --git a/packages/app/src/utils/keyboard-ime.test.ts b/packages/app/src/utils/keyboard-ime.test.ts new file mode 100644 index 000000000..e505f4b63 --- /dev/null +++ b/packages/app/src/utils/keyboard-ime.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { isImeComposingKeyboardEvent } from "./keyboard-ime"; + +describe("isImeComposingKeyboardEvent", () => { + it("ignores events while IME composition is active", () => { + expect( + isImeComposingKeyboardEvent({ + isComposing: true, + keyCode: 13, + } as KeyboardEvent), + ).toBe(true); + }); + + it("ignores Chromium IME fallback events with keyCode 229", () => { + expect( + isImeComposingKeyboardEvent({ + isComposing: false, + keyCode: 229, + } as KeyboardEvent), + ).toBe(true); + }); + + it("keeps regular keyboard events eligible for shortcuts", () => { + expect( + isImeComposingKeyboardEvent({ + isComposing: false, + keyCode: 13, + } as KeyboardEvent), + ).toBe(false); + }); +}); diff --git a/packages/app/src/utils/keyboard-ime.ts b/packages/app/src/utils/keyboard-ime.ts new file mode 100644 index 000000000..7d1d7edfc --- /dev/null +++ b/packages/app/src/utils/keyboard-ime.ts @@ -0,0 +1,5 @@ +export function isImeComposingKeyboardEvent( + event: Pick, +): boolean { + return event.isComposing || event.keyCode === 229; +} diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.test.ts b/packages/server/src/server/agent/providers/generic-acp-agent.test.ts new file mode 100644 index 000000000..cd05d1f3a --- /dev/null +++ b/packages/server/src/server/agent/providers/generic-acp-agent.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, test, vi } from "vitest"; + +import { createTestLogger } from "../../../test-utils/test-logger.js"; + +const mockState = vi.hoisted(() => ({ + superConstructorOptions: [] as unknown[], +})); + +vi.mock("./acp-agent.js", () => ({ + ACPAgentClient: class ACPAgentClient { + readonly provider: string; + + constructor(options: unknown) { + this.provider = "acp"; + mockState.superConstructorOptions.push(options); + } + }, +})); + +import { GenericACPAgentClient } from "./generic-acp-agent.js"; + +describe("GenericACPAgentClient", () => { + test("passes the custom command only as defaultCommand", () => { + new GenericACPAgentClient({ + logger: createTestLogger(), + command: ["hermes", "acp"], + env: { + HERMES_LOG: "info", + }, + }); + + expect(mockState.superConstructorOptions).toEqual([ + { + provider: "acp", + logger: expect.any(Object), + runtimeSettings: { + env: { + HERMES_LOG: "info", + }, + }, + defaultCommand: ["hermes", "acp"], + }, + ]); + }); +}); diff --git a/packages/server/src/server/agent/providers/generic-acp-agent.ts b/packages/server/src/server/agent/providers/generic-acp-agent.ts index ea9423df9..53b2f0f1c 100644 --- a/packages/server/src/server/agent/providers/generic-acp-agent.ts +++ b/packages/server/src/server/agent/providers/generic-acp-agent.ts @@ -21,10 +21,6 @@ export class GenericACPAgentClient extends ACPAgentClient { provider: "acp", logger: options.logger, runtimeSettings: { - command: { - mode: "replace", - argv: options.command, - }, env: options.env, }, defaultCommand: options.command as [string, ...string[]],