Fix duplicate command args for generic ACP providers (#444)
* Fix duplicate command args for generic ACP providers * Fix desktop IME enter handling
This commit is contained in:
@@ -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<MessageInputRef, MessageInputProps>(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) {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,5 @@
|
||||
export function isImeComposingKeyboardEvent(
|
||||
event: Pick<KeyboardEvent, "isComposing" | "keyCode">,
|
||||
): boolean {
|
||||
return event.isComposing || event.keyCode === 229;
|
||||
}
|
||||
@@ -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"],
|
||||
},
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -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[]],
|
||||
|
||||
Reference in New Issue
Block a user