diff --git a/packages/app/e2e/browser/terminal-dictation.spec.ts b/packages/app/e2e/browser/terminal-dictation.spec.ts index 977e8bd827..aafd69e049 100644 --- a/packages/app/e2e/browser/terminal-dictation.spec.ts +++ b/packages/app/e2e/browser/terminal-dictation.spec.ts @@ -276,6 +276,52 @@ test.describe("Terminal dictation", () => { expect(await received(bracketed.id, "bracketed")).toBe("first second "); }); + test("the dictation pill keeps the tab row usable in a narrow split", async ({ page }) => { + const ws = await setup(page, { transcript: "narrow split words" }); + const left = await createReceiver("bracketed", "dictation-narrow-left"); + const right = await createReceiver("bracketed", "dictation-narrow-right"); + + await harness.openTerminal(page, { terminalId: left.id }); + await runWorkspaceActionFromCommandCenter(page, "Split pane right"); + await visibleTestId(page, `workspace-tab-terminal_${right.id}`).click(); + await runWorkspaceActionFromCommandCenter(page, "Move tab right"); + // Narrow enough that the pill's extra width decides whether the tab still fits. + await page.setViewportSize({ width: 1000, height: 900 }); + const pane = splitChild(page, right); + + const tab = visibleTestId(pane, `workspace-tab-terminal_${right.id}`); + const newTabButton = visibleTestId(pane, "workspace-new-tab-button"); + const inlineNewTabButton = pane + .getByTestId("workspace-tabs-scroll") + .getByTestId("workspace-new-tab-button"); + // Either the tab fits beside the inline new-tab button, or the row has switched to + // its overflow fallback: tabs scroll and the new-tab button moves to the toolbar. + // The button is never clipped by the tab scroller. + const expectTabsFit = async () => { + await expect(inlineNewTabButton).toHaveCount(1); + await expect(newTabButton).toBeInViewport({ ratio: 1 }); + await expect(tab).toBeInViewport({ ratio: 1 }); + }; + + await expectTabsFit(); + await startFromToolbar(page, ws, pane); + await expect(visibleTestId(pane, "terminal-dictation-pill")).toBeVisible(); + await expect(newTabButton).toBeInViewport({ ratio: 1 }); + await expect(tab).toBeInViewport({ ratio: 1 }); + + // The failed pill carries the error text, which leaves too little room for the tab. + ws.failNextFinish("Synthetic transcription failure"); + await visibleTestId(pane, "terminal-dictation-confirm").click(); + await expect(visibleTestId(pane, "terminal-dictation-retry")).toBeVisible(); + await expect(inlineNewTabButton).toHaveCount(0); + await expect(newTabButton).toBeInViewport({ ratio: 1 }); + + await visibleTestId(pane, "terminal-dictation-retry").click(); + await expectReceived(right.id, "bracketed", "narrow split words "); + await expect(visibleTestId(pane, "terminal-toolbar-dictation")).toBeVisible(); + await expectTabsFit(); + }); + test.describe("on macOS", () => { test.use({ userAgent: MAC_USER_AGENT }); @@ -429,9 +475,76 @@ test.describe("Terminal dictation", () => { await expectNothingReceived(page, [first, "bracketed"], [second, "bracketed"]); }); + // Fails the first confirm, then retries with the final reply held, so the retry is + // still processing when the test acts on it. + async function startHeldRetry(page: Page, ws: DictationWsHarness) { + ws.failNextFinish("Synthetic transcription failure"); + await startFromToolbar(page, ws); + await visibleTestId(page, "terminal-dictation-confirm").click(); + await expect(visibleTestId(page, "terminal-dictation-retry")).toBeVisible(); + ws.holdFinal(); + await visibleTestId(page, "terminal-dictation-retry").click(); + await expect(visibleTestId(page, "terminal-dictation-retry")).toHaveCount(0); + await expect.poll(() => ws.hasHeldFinal()).toBe(true); + } + + test("cancelling a retry drops its late transcript", async ({ page }) => { + const ws = await setup(page, { transcript: "cancelled retry words" }); + const terminal = await createReceiver("bracketed", "dictation-retry-cancel"); + + await harness.openTerminal(page, { terminalId: terminal.id }); + await startHeldRetry(page, ws); + await visibleTestId(page, "terminal-dictation-cancel").click(); + await expect(visibleTestId(page, "terminal-toolbar-dictation")).toBeVisible(); + ws.releaseFinal(); + + await expectNothingReceived(page, [terminal, "bracketed"]); + await expect(visibleTestId(page, "terminal-toolbar-dictation")).toBeVisible(); + }); + + test("hiding a retrying terminal and returning before its reply drops the transcript", async ({ + page, + }) => { + const ws = await setup(page, { transcript: "hidden retry words" }); + const first = await createReceiver("bracketed", "dictation-retry-hide-a"); + const second = await createReceiver("bracketed", "dictation-retry-hide-b"); + + await harness.openTerminal(page, { terminalId: first.id }); + await startHeldRetry(page, ws); + await selectTerminalTab(page, second); + await expect(visibleTestId(page, "terminal-toolbar-dictation")).toBeVisible(); + // Back on the retained pane, which is presented and still the recording owner. + await selectTerminalTab(page, first); + await expect(visibleTestId(page, "terminal-toolbar-dictation")).toBeVisible(); + ws.releaseFinal(); + + await expectNothingReceived(page, [first, "bracketed"], [second, "bracketed"]); + }); + + test.describe("on compact web on macOS", () => { + test.use({ userAgent: MAC_USER_AGENT }); + + // Compact web turns shortcuts off except terminal dictation's Cmd+D. + test("Cmd+D starts and then confirms dictation in the focused terminal", async ({ page }) => { + const ws = await setup(page, { transcript: "compact shortcut words" }); + const terminal = await createReceiver("bracketed", "dictation-compact-shortcut"); + + await openCompact(page, terminal); + await terminalSurface(page).click(); + const chunk = ws.nextChunk(); + await terminalInput(page).press("Meta+d"); + await expect(visibleTestId(page, "terminal-dictation-overlay")).toBeVisible(); + await chunk; + await expect(terminalInput(page)).toBeFocused(); + await terminalInput(page).press("Meta+d"); + + await expectReceived(terminal.id, "bracketed", "compact shortcut words "); + await expect(page.getByTestId("terminal-dictation-overlay")).toHaveCount(0); + await expect(visibleTestId(page, "terminal-dictation")).toBeVisible(); + }); + }); + test.describe("on compact web", () => { - // Keyboard shortcuts are off on compact form factors (keyboardShortcutsAvailable), so - // the key-bar mic is how compact web dictates. test("the key-bar mic shows the overlay and confirm delivers", async ({ page }) => { const ws = await setup(page, { transcript: "compact words" }); const terminal = await createReceiver("bracketed", "dictation-compact-confirm"); diff --git a/packages/app/e2e/support/helpers/dictation-ws.ts b/packages/app/e2e/support/helpers/dictation-ws.ts index 1496c13052..6dcaf5a778 100644 --- a/packages/app/e2e/support/helpers/dictation-ws.ts +++ b/packages/app/e2e/support/helpers/dictation-ws.ts @@ -152,6 +152,8 @@ export interface DictationWsHarness { setTranscript(text: string): void; /** Keeps the next final reply until `releaseFinal`. */ holdFinal(): void; + /** True once the held finish has arrived and its final reply is waiting. */ + hasHeldFinal(): boolean; releaseFinal(): void; /** Answers the next finish with a stream error instead of a final. */ failNextFinish(error: string): void; @@ -263,6 +265,7 @@ export async function installDictationWsHarness( holdFinal: () => { holdNextFinal = true; }, + hasHeldFinal: () => heldFinal !== null, releaseFinal: () => { const reply = heldFinal; heldFinal = null; diff --git a/packages/app/src/audio/audio-engine.native.ts b/packages/app/src/audio/audio-engine.native.ts index ed9f15a765..f14ede49f7 100644 --- a/packages/app/src/audio/audio-engine.native.ts +++ b/packages/app/src/audio/audio-engine.native.ts @@ -5,18 +5,39 @@ import type { AudioPlaybackSource, } from "./audio-engine-types"; +import type * as TwoWayAudio from "@getpaseo/expo-two-way-audio"; import { createAudioPlayer, setAudioModeAsync } from "expo-audio"; import { File, Paths } from "expo-file-system"; +import { AttemptCancelledError } from "@/utils/attempt-guard"; import { createPlaybackQueue } from "./playback"; import { playFile } from "./file-playback"; import { playPcm16 } from "./pcm"; +export interface NativeCaptureStartOptions { + /** + * Aborting cancels a start still waiting on microphone permission or native + * initialization: it rejects with AttemptCancelledError and never starts capture. + */ + signal?: AbortSignal; +} + +export interface NativeAudioEngine extends AudioEngine { + startCapture(options?: NativeCaptureStartOptions): Promise; +} + export function createAudioEngine( callbacks: AudioEngineCallbacks, options?: AudioEngineOptions, -): AudioEngine { - const native = require("@getpaseo/expo-two-way-audio"); +): NativeAudioEngine { + return createNativeAudioEngine(require("@getpaseo/expo-two-way-audio"), callbacks, options); +} +/** The engine over a given native module; tests pass a fake of the OS audio APIs. */ +export function createNativeAudioEngine( + native: typeof TwoWayAudio, + callbacks: AudioEngineCallbacks, + options?: AudioEngineOptions, +): NativeAudioEngine { const refs: { initialized: boolean; captureActive: boolean; @@ -72,6 +93,9 @@ export function createAudioEngine( ); async function ensureInitialized(): Promise { + if (refs.destroyed) { + throw new AttemptCancelledError("Audio engine was destroyed"); + } if (refs.initialized) { return; } @@ -79,9 +103,20 @@ export function createAudioEngine( if (!success) { throw new Error("expo-two-way-audio: native initialize() returned false"); } + if (refs.destroyed) { + // Destroyed while initializing: destroy() saw nothing to tear down. + native.tearDown(); + throw new AttemptCancelledError("Audio engine was destroyed"); + } refs.initialized = true; } + function throwIfStartCancelled(signal: AbortSignal | undefined): void { + if (refs.destroyed || signal?.aborted) { + throw new AttemptCancelledError("Audio capture start was cancelled"); + } + } + /** * Release the OS audio session as soon as we are neither capturing nor playing. * Holding it keeps the user's background music paused — on iOS the non-mixing @@ -183,14 +218,20 @@ export function createAudioEngine( interruptionSubscription.remove(); }, - async startCapture() { + async startCapture(startOptions) { if (refs.captureActive) { return; } + const signal = startOptions?.signal; try { + // Permission and initialization can wait on the user or the OS; a destroy or + // an abort in that time must not leave capture running with no owner. + throwIfStartCancelled(signal); await ensureMicrophonePermission(); + throwIfStartCancelled(signal); await ensureInitialized(); + throwIfStartCancelled(signal); const isRecording = options?.captureMode === "dictation" ? native.startDictation() @@ -202,6 +243,10 @@ export function createAudioEngine( } refs.captureActive = true; } catch (error) { + if (error instanceof AttemptCancelledError) { + releaseSessionIfIdle(); + throw error; + } const wrapped = error instanceof Error ? error : new Error(String(error)); callbacks.onError?.(wrapped); throw wrapped; diff --git a/packages/app/src/hooks/use-dictation-audio-source.native.test.tsx b/packages/app/src/hooks/use-dictation-audio-source.native.test.tsx new file mode 100644 index 0000000000..cbbf0b609d --- /dev/null +++ b/packages/app/src/hooks/use-dictation-audio-source.native.test.tsx @@ -0,0 +1,204 @@ +/** + * @vitest-environment jsdom + */ +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type * as TwoWayAudio from "@getpaseo/expo-two-way-audio"; +import type { AudioEngineCallbacks, AudioEngineOptions } from "@/audio/audio-engine-types"; +import type { UseDictationOptions } from "./use-dictation.shared"; + +// The actual native audio source and engine under useDictation; only the OS audio +// module is fake, with its permission and initialize calls settled by the test. +const os = vi.hoisted(() => { + const state = { + calls: [] as string[], + capturing: false, + initialized: false, + permission: null as null | ((granted: boolean) => void), + initialize: null as null | (() => void), + holdPermission: false, + holdInitialize: false, + }; + return state; +}); + +function createFakeNativeModule() { + return { + addExpoTwoWayAudioEventListener: () => ({ remove: () => {} }), + initialize: () => { + os.calls.push("initialize"); + return new Promise((resolve) => { + const finish = () => { + os.initialized = true; + resolve(true); + }; + if (os.holdInitialize) { + os.initialize = finish; + } else { + finish(); + } + }); + }, + getMicrophonePermissionsAsync: async () => ({ granted: !os.holdPermission }), + requestMicrophonePermissionsAsync: () => + new Promise<{ granted: boolean }>((resolve) => { + os.permission = (granted) => resolve({ granted }); + }), + startDictation: () => { + os.calls.push("startDictation"); + os.capturing = true; + return true; + }, + toggleRecording: (value: boolean) => { + os.calls.push(`toggleRecording:${value}`); + os.capturing = value; + return value; + }, + tearDown: () => { + os.calls.push("tearDown"); + os.capturing = false; + os.initialized = false; + }, + releaseAudioSession: () => {}, + }; +} + +// The engine's file playback is not exercised; these keep Expo's runtime out of Node. +vi.mock("expo-audio", () => ({ createAudioPlayer: vi.fn(), setAudioModeAsync: vi.fn() })); +vi.mock("expo-file-system", () => ({ File: vi.fn(), Paths: {} })); +vi.mock("@/audio", async () => { + const { createNativeAudioEngine } = await import("@/audio/audio-engine.native"); + return { + createAudioEngine: (callbacks: AudioEngineCallbacks, options?: AudioEngineOptions) => + createNativeAudioEngine( + createFakeNativeModule() as unknown as typeof TwoWayAudio, + callbacks, + options, + ), + }; +}); +vi.mock("@/hooks/use-dictation-audio-source", () => import("./use-dictation-audio-source.native")); + +const { useDictation } = await import("./use-dictation"); + +function renderDictation() { + const onError = vi.fn(); + const rendered = renderHook(() => + useDictation({ + client: null as UseDictationOptions["client"], + onTranscript: () => {}, + onError, + }), + ); + return { ...rendered, onError }; +} + +async function flush(): Promise { + await act(async () => { + for (let i = 0; i < 5; i += 1) { + await Promise.resolve(); + } + }); +} + +beforeEach(() => { + os.calls = []; + os.capturing = false; + os.initialized = false; + os.permission = null; + os.initialize = null; + os.holdPermission = false; + os.holdInitialize = false; +}); + +describe("native dictation source pending start", () => { + it("starts capture when the start is not cancelled", async () => { + const { result, onError } = renderDictation(); + + await act(async () => { + await result.current.startDictation(); + }); + + expect(os.calls).toEqual(["initialize", "startDictation"]); + expect(os.capturing).toBe(true); + expect(result.current.isRecording).toBe(true); + expect(onError).not.toHaveBeenCalled(); + }); + + it("does not start capture when permission is granted after the owner unmounts", async () => { + os.holdPermission = true; + const { result, unmount, onError } = renderDictation(); + + let start!: Promise; + act(() => { + start = result.current.startDictation(); + }); + await flush(); + expect(os.permission).not.toBeNull(); + unmount(); + + await act(async () => { + os.permission?.(true); + await start; + }); + + expect(os.calls).not.toContain("startDictation"); + expect(os.capturing).toBe(false); + expect(os.initialized).toBe(false); + expect(onError).not.toHaveBeenCalled(); + }); + + it("does not start capture when permission is granted after a cancel, and starts again", async () => { + os.holdPermission = true; + const { result, onError } = renderDictation(); + + let start!: Promise; + act(() => { + start = result.current.startDictation(); + }); + await flush(); + await act(async () => { + await result.current.cancelDictation(); + }); + await act(async () => { + os.permission?.(true); + await start; + }); + + expect(os.calls).not.toContain("startDictation"); + expect(os.capturing).toBe(false); + expect(result.current.status).toBe("idle"); + expect(onError).not.toHaveBeenCalled(); + + os.holdPermission = false; + await act(async () => { + await result.current.startDictation(); + }); + expect(os.calls.filter((call) => call === "startDictation")).toHaveLength(1); + expect(os.capturing).toBe(true); + expect(result.current.isRecording).toBe(true); + }); + + it("tears the native session down when initialization finishes after the owner unmounts", async () => { + os.holdInitialize = true; + const { result, unmount, onError } = renderDictation(); + + let start!: Promise; + act(() => { + start = result.current.startDictation(); + }); + await flush(); + expect(os.initialize).not.toBeNull(); + unmount(); + + await act(async () => { + os.initialize?.(); + await start; + }); + + expect(os.calls).toEqual(["initialize", "tearDown"]); + expect(os.capturing).toBe(false); + expect(os.initialized).toBe(false); + expect(onError).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app/src/hooks/use-dictation-audio-source.native.ts b/packages/app/src/hooks/use-dictation-audio-source.native.ts index c34f9bbd02..89e5eadbf2 100644 --- a/packages/app/src/hooks/use-dictation-audio-source.native.ts +++ b/packages/app/src/hooks/use-dictation-audio-source.native.ts @@ -3,6 +3,7 @@ import { Buffer } from "buffer"; import { useState } from "react"; import { createAudioEngine } from "@/audio"; +import { AttemptCancelledError } from "@/utils/attempt-guard"; import type { DictationAudioSource, @@ -16,6 +17,9 @@ export function useDictationAudioSource(config: DictationAudioSourceConfig): Dic const [volume, setVolume] = useState(0); const [isPaused, setIsPaused] = useState(false); const engineRef = useRef | null>(null); + // The start in flight. stop() and unmount abort it, so a permission granted or an + // initialization finished afterwards cannot start capture nobody will stop. + const startAbortRef = useRef(null); const getOrCreateEngine = useCallback(() => { if (engineRef.current) { @@ -50,18 +54,28 @@ export function useDictationAudioSource(config: DictationAudioSourceConfig): Dic }, [config.onPcmSegment, config.onError, config.onInterruption]); const start = useCallback(async () => { + startAbortRef.current?.abort(); + const controller = new AbortController(); + startAbortRef.current = controller; const engine = getOrCreateEngine(); await engine.initialize(); - await engine.startCapture(); + if (controller.signal.aborted) { + throw new AttemptCancelledError("Dictation start was cancelled"); + } + await engine.startCapture({ signal: controller.signal }); }, [getOrCreateEngine]); const stop = useCallback(async () => { + startAbortRef.current?.abort(); + startAbortRef.current = null; await engineRef.current?.stopCapture(); setVolume(0); }, []); useEffect(() => { return () => { + startAbortRef.current?.abort(); + startAbortRef.current = null; const engine = engineRef.current; engineRef.current = null; void engine?.destroy().catch(() => undefined); diff --git a/packages/app/src/hooks/use-dictation.test.tsx b/packages/app/src/hooks/use-dictation.test.tsx index 06c990a645..f9c108196a 100644 --- a/packages/app/src/hooks/use-dictation.test.tsx +++ b/packages/app/src/hooks/use-dictation.test.tsx @@ -33,18 +33,43 @@ vi.mock("@/hooks/use-dictation-audio-source", () => ({ const { useDictation } = await import("./use-dictation"); function createFakeClient() { + let rawMessageHandler: ((message: unknown) => void) | null = null; return { isConnected: true, startDictationStream: vi.fn(async () => {}), - sendDictationStreamChunk: vi.fn(), + // The daemon acknowledges each chunk, which lets finish() stop waiting for delivery. + sendDictationStreamChunk: vi.fn((dictationId: string, seq: number) => { + rawMessageHandler?.({ type: "dictation_stream_ack", payload: { dictationId, ackSeq: seq } }); + }), finishDictationStream: vi.fn(async (dictationId: string) => ({ dictationId, text: "" })), cancelDictationStream: vi.fn(), - subscribeRawMessages: vi.fn(() => () => {}), + subscribeRawMessages: vi.fn((handler: (message: unknown) => void) => { + rawMessageHandler = handler; + return () => { + rawMessageHandler = null; + }; + }), subscribeConnectionStatus: vi.fn(() => () => {}), on: vi.fn(() => () => {}), }; } +/** A finish reply the test settles, standing in for a transcription still in progress. */ +function holdFinish(client: ReturnType) { + const held: { + resolve: (text: string) => void; + reject: (error: Error) => void; + } = { resolve: () => {}, reject: () => {} }; + client.finishDictationStream.mockImplementationOnce( + (dictationId: string) => + new Promise((resolve, reject) => { + held.resolve = (text) => resolve({ dictationId, text }); + held.reject = reject; + }), + ); + return held; +} + function renderDictation(client: ReturnType) { const onError = vi.fn(); const onTranscript = vi.fn(); @@ -201,3 +226,147 @@ describe("useDictation pending startup", () => { expect(client.sendDictationStreamChunk).not.toHaveBeenCalled(); }); }); + +describe("useDictation retry", () => { + // Records one segment, then lets confirm fail so the attempt waits on retry. + async function failFirstConfirm( + client: ReturnType, + result: { current: ReturnType }, + ) { + act(() => { + void result.current.startDictation(); + }); + await act(async () => { + audioSource.settleStart?.resolve(); + }); + await flush(); + act(() => { + audioSource.onPcmSegment?.("AAAA"); + }); + client.finishDictationStream.mockRejectedValueOnce(new Error("Synthetic failure")); + await act(async () => { + await result.current.confirmDictation(); + }); + expect(result.current.status).toBe("failed"); + } + + it("delivers the transcript of a retry that is not cancelled", async () => { + const client = createFakeClient(); + const { result, onTranscript } = renderDictation(client); + await failFirstConfirm(client, result); + + client.finishDictationStream.mockImplementationOnce(async (dictationId: string) => ({ + dictationId, + text: "retried words", + })); + await act(async () => { + await result.current.retryFailedDictation(); + }); + + expect(onTranscript).toHaveBeenCalledTimes(1); + expect(onTranscript.mock.calls[0]?.[0]).toBe("retried words"); + expect(result.current.status).toBe("idle"); + }); + + it("drops the transcript of a retry cancelled before its reply", async () => { + const client = createFakeClient(); + const { result, onTranscript } = renderDictation(client); + await failFirstConfirm(client, result); + + const held = holdFinish(client); + act(() => { + void result.current.retryFailedDictation(); + }); + await flush(); + expect(result.current.status).toBe("uploading"); + await act(async () => { + await result.current.cancelDictation(); + }); + expect(result.current.status).toBe("idle"); + + await act(async () => { + held.resolve("cancelled retry words"); + }); + await flush(); + + expect(onTranscript).not.toHaveBeenCalled(); + expect(result.current.status).toBe("idle"); + }); + + it("drops the failure of a retry cancelled before its reply", async () => { + const client = createFakeClient(); + const { result, onError } = renderDictation(client); + await failFirstConfirm(client, result); + onError.mockClear(); + + const held = holdFinish(client); + act(() => { + void result.current.retryFailedDictation(); + }); + await flush(); + await act(async () => { + await result.current.cancelDictation(); + }); + + await act(async () => { + held.reject(new Error("Late failure")); + }); + await flush(); + + expect(onError).not.toHaveBeenCalled(); + expect(result.current.status).toBe("idle"); + expect(result.current.error).toBeNull(); + }); + + it("keeps a newer recording when a cancelled retry's reply arrives", async () => { + const client = createFakeClient(); + const { result, onTranscript } = renderDictation(client); + await failFirstConfirm(client, result); + + const held = holdFinish(client); + act(() => { + void result.current.retryFailedDictation(); + }); + await flush(); + await act(async () => { + await result.current.cancelDictation(); + }); + act(() => { + void result.current.startDictation(); + }); + await act(async () => { + audioSource.settleStart?.resolve(); + }); + await flush(); + expect(result.current.status).toBe("recording"); + + await act(async () => { + held.reject(new Error("Late failure")); + }); + await flush(); + + expect(onTranscript).not.toHaveBeenCalled(); + expect(result.current.status).toBe("recording"); + expect(result.current.isRecording).toBe(true); + }); + + it("drops the transcript of a retry when the owner unmounts before its reply", async () => { + const client = createFakeClient(); + const { result, unmount, onTranscript } = renderDictation(client); + await failFirstConfirm(client, result); + + const held = holdFinish(client); + act(() => { + void result.current.retryFailedDictation(); + }); + await flush(); + unmount(); + + await act(async () => { + held.resolve("unmounted retry words"); + }); + await flush(); + + expect(onTranscript).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/app/src/hooks/use-dictation.ts b/packages/app/src/hooks/use-dictation.ts index 44dfc6d6b0..0d2b825ae4 100644 --- a/packages/app/src/hooks/use-dictation.ts +++ b/packages/app/src/hooks/use-dictation.ts @@ -433,7 +433,8 @@ export function useDictation(options: UseDictationOptions): UseDictationResult { attemptGuardRef.current.assertCurrent(attemptId); handleStreamingTranscriptionSuccess(transcriptText, generateMessageId()); } catch (err) { - if (err instanceof Error && err.name === "AttemptCancelledError") { + // A failure that lands after a cancel belongs to an abandoned attempt. + if (!attemptGuardRef.current.isCurrent(attemptId)) { return; } handleDictationFailure(err); @@ -450,9 +451,12 @@ export function useDictation(options: UseDictationOptions): UseDictationResult { ]); const retryFailedDictation = useCallback(async () => { - if (!senderRef.current?.hasSegments()) { + if (isProcessingRef.current || !senderRef.current?.hasSegments()) { return; } + // Same attempt rule as confirm: a cancel, unmount or newer attempt while the + // final transcript is pending makes both its result and its failure stale. + const attemptId = attemptGuardRef.current.next(); setError(null); setStatus("uploading"); setIsProcessing(true); @@ -465,9 +469,10 @@ export function useDictation(options: UseDictationOptions): UseDictationResult { senderRef.current.resetStreamForReplay(); const finalSeq = senderRef.current.getFinalSeq(); const text = await ensureFinalTranscript(finalSeq); + attemptGuardRef.current.assertCurrent(attemptId); handleStreamingTranscriptionSuccess(text, generateMessageId()); } catch (err) { - if (err instanceof Error && err.name === "AttemptCancelledError") { + if (!attemptGuardRef.current.isCurrent(attemptId)) { return; } handleDictationFailure(err); diff --git a/packages/app/src/hooks/use-keyboard-shortcuts.ts b/packages/app/src/hooks/use-keyboard-shortcuts.ts index 6f332b0a98..b95a7ae078 100644 --- a/packages/app/src/hooks/use-keyboard-shortcuts.ts +++ b/packages/app/src/hooks/use-keyboard-shortcuts.ts @@ -30,7 +30,7 @@ import { useOpenAddProject } from "@/hooks/use-open-add-project"; import { useStableEvent } from "@/hooks/use-stable-event"; import { useKeyboardShortcutOverrides } from "@/hooks/use-keyboard-shortcut-overrides"; import { isNative } from "@/constants/platform"; -import { keyboardShortcutsAvailable } from "@/keyboard/availability"; +import { dispatchableShortcutBindings, keyboardShortcutsAvailable } from "@/keyboard/availability"; import { getDesktopHost, isElectronRuntime } from "@/desktop/host"; import { isImeComposingKeyboardEvent } from "@/utils/keyboard-ime"; import { buildOpenProjectRoute } from "@/utils/host-routes"; @@ -66,6 +66,10 @@ export function useKeyboardShortcuts({ const { overrides } = useKeyboardShortcutOverrides(); const bindings = useMemo(() => buildEffectiveBindings(overrides), [overrides]); const shortcutsAvailable = keyboardShortcutsAvailable({ isNative, isCompact: isMobile }); + const dispatchBindings = useMemo( + () => dispatchableShortcutBindings(bindings, { isNative, isCompact: isMobile }), + [bindings, isMobile], + ); const isDesktopApp = getIsElectronRuntime(); const isMac = getShortcutOs() === "mac"; const chordStateRef = useRef({ @@ -269,7 +273,7 @@ export function useKeyboardShortcuts({ }; publishBrowserShortcutPolicy(); }, - bindings, + bindings: dispatchBindings, }); chordStateRef.current = result.nextChordState; if ( @@ -366,6 +370,26 @@ export function useKeyboardShortcuts({ }); }); + // Compact web dispatches only its `compact` bindings, so it skips the badge and settings + // handling above, which belong to the full set. Overlays keep their own listener. + const handleCompactKeyDown = useStableEvent((event: KeyboardEvent) => { + if (!shouldHandle() || isImeComposingKeyboardEvent(event)) { + return; + } + const store = useKeyboardShortcutsStore.getState(); + if (store.capturingShortcut) { + return; + } + resolveAndPerformShortcut({ + event, + focusScope: resolveKeyboardFocusScope({ + target: event.target, + commandCenterOpen: store.commandCenterOpen, + }), + domEvent: event, + }); + }); + const handleKeyUp = useStableEvent((event: KeyboardEvent) => { const key = event.key ?? ""; if (key === badgeModifierKey) { @@ -386,6 +410,14 @@ export function useKeyboardShortcuts({ }); }); + useEffect(() => { + if (!enabled || shortcutsAvailable || dispatchBindings.length === 0) return; + window.addEventListener("keydown", handleCompactKeyDown, true); + return () => { + window.removeEventListener("keydown", handleCompactKeyDown, true); + }; + }, [dispatchBindings, enabled, handleCompactKeyDown, shortcutsAvailable]); + useEffect(() => { if (!enabled) return; if (!shortcutsAvailable) return; diff --git a/packages/app/src/keyboard/availability.test.ts b/packages/app/src/keyboard/availability.test.ts index 282a879006..f96b8ceff0 100644 --- a/packages/app/src/keyboard/availability.test.ts +++ b/packages/app/src/keyboard/availability.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { keyboardShortcutsAvailable } from "./availability"; +import { dispatchableShortcutBindings, keyboardShortcutsAvailable } from "./availability"; describe("keyboardShortcutsAvailable", () => { it("matches the environments where the shortcut dispatcher runs", () => { @@ -9,3 +9,31 @@ describe("keyboardShortcutsAvailable", () => { expect(keyboardShortcutsAvailable({ isNative: true, isCompact: true })).toBe(false); }); }); + +describe("dispatchableShortcutBindings", () => { + const bindings = [ + { id: "everywhere-but-compact" }, + { id: "compact-too", compact: true as const }, + ]; + + it("dispatches every binding on wide web", () => { + expect(dispatchableShortcutBindings(bindings, { isNative: false, isCompact: false })).toEqual( + bindings, + ); + }); + + it("dispatches only the compact bindings on compact web", () => { + expect( + dispatchableShortcutBindings(bindings, { isNative: false, isCompact: true }).map( + (binding) => binding.id, + ), + ).toEqual(["compact-too"]); + }); + + it("dispatches nothing on native, compact or not", () => { + expect(dispatchableShortcutBindings(bindings, { isNative: true, isCompact: false })).toEqual( + [], + ); + expect(dispatchableShortcutBindings(bindings, { isNative: true, isCompact: true })).toEqual([]); + }); +}); diff --git a/packages/app/src/keyboard/availability.ts b/packages/app/src/keyboard/availability.ts index ec3957f58f..bf286950f4 100644 --- a/packages/app/src/keyboard/availability.ts +++ b/packages/app/src/keyboard/availability.ts @@ -13,6 +13,24 @@ export function keyboardShortcutsAvailable({ return !native && !isCompact; } +/** + * The bindings the shortcut dispatcher listens for. Native has no dispatcher. Compact web + * keeps only the bindings marked `compact`; everything else about shortcuts, hints and + * help included, follows `keyboardShortcutsAvailable`. + */ +export function dispatchableShortcutBindings( + bindings: readonly Binding[], + { isNative: native, isCompact }: KeyboardShortcutEnvironment, +): readonly Binding[] { + if (native) { + return []; + } + if (isCompact) { + return bindings.filter((binding) => binding.compact === true); + } + return bindings; +} + export function useKeyboardShortcutsAvailable(): boolean { const isCompact = useIsCompactFormFactor(); return keyboardShortcutsAvailable({ isNative, isCompact }); diff --git a/packages/app/src/keyboard/keyboard-shortcuts.test.ts b/packages/app/src/keyboard/keyboard-shortcuts.test.ts index c7239ff626..6168e229d1 100644 --- a/packages/app/src/keyboard/keyboard-shortcuts.test.ts +++ b/packages/app/src/keyboard/keyboard-shortcuts.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "vitest"; import { formatShortcut } from "@/utils/format-shortcut"; import { keyboardEventToComboString } from "./shortcut-string"; +import { dispatchableShortcutBindings } from "./availability"; import { buildKeyboardShortcutHelpSections, buildEffectiveBindings, @@ -1422,3 +1423,61 @@ describe("shortcut capture completion", () => { expect(isShortcutCaptureComplete("show-shortcuts", ["Ctrl+0"])).toBe(true); }); }); + +describe("compact web shortcuts", () => { + const compactBindings = dispatchableShortcutBindings(buildEffectiveBindings({}), { + isNative: false, + isCompact: true, + }); + + it("keeps only terminal dictation's Cmd+D", () => { + expect(compactBindings.map((binding) => binding.id)).toEqual([ + "terminal-dictation-toggle-cmd-d-mac", + ]); + }); + + it("routes Cmd+D in a focused terminal on macOS", () => { + const result = resolveShortcut({ + event: { key: "d", code: "KeyD", metaKey: true }, + context: { isMac: true, focusScope: "terminal" }, + bindings: compactBindings, + }); + expect(result.match?.action).toBe("terminal.dictation-toggle"); + }); + + it("leaves Ctrl+D off macOS to the PTY", () => { + const result = resolveShortcut({ + event: { key: "d", code: "KeyD", ctrlKey: true }, + context: { isMac: false, focusScope: "terminal" }, + bindings: compactBindings, + }); + expect(result.match).toBeNull(); + expect(result.preventDefault).toBe(false); + }); + + it.each([ + { + name: "the composer's Cmd+D", + event: { key: "d", code: "KeyD", metaKey: true }, + focusScope: "message-input" as const, + }, + { + name: "the command center", + event: { key: "k", code: "KeyK", metaKey: true }, + focusScope: "other" as const, + }, + { + name: "agent interrupt", + event: { key: "Escape", code: "Escape" }, + focusScope: "message-input" as const, + }, + ])("keeps $name off", ({ event, focusScope }) => { + const result = resolveShortcut({ + event, + context: { isMac: true, focusScope }, + bindings: compactBindings, + }); + expect(result.match).toBeNull(); + expect(result.preventDefault).toBe(false); + }); +}); diff --git a/packages/app/src/keyboard/keyboard-shortcuts.ts b/packages/app/src/keyboard/keyboard-shortcuts.ts index 90b376c1e4..bbada864c0 100644 --- a/packages/app/src/keyboard/keyboard-shortcuts.ts +++ b/packages/app/src/keyboard/keyboard-shortcuts.ts @@ -111,6 +111,11 @@ interface ShortcutBinding { preventDefault?: boolean; stopPropagation?: boolean; help?: ShortcutHelp; + /** + * Also dispatched on compact web, where every other shortcut is off. Only for a key a + * hardware keyboard user at a narrow width has no other way to reach. + */ + compact?: true; } export interface ParsedShortcutBinding extends ShortcutBinding { @@ -1144,6 +1149,8 @@ const SHORTCUT_BINDINGS: readonly ShortcutBinding[] = [ action: "terminal.dictation-toggle", combo: "Cmd+D", when: { mac: true, commandCenter: false, focusScope: "terminal" }, + // A narrow browser window has no tab bar mic, only the key bar's. + compact: true, help: { id: "terminal-dictation-toggle", section: "agent-input", diff --git a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx index c5389fae16..2fa403f0b0 100644 --- a/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx +++ b/packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx @@ -105,9 +105,12 @@ const PANE_SPLIT_ACTIONS_RESERVED_WIDTH = PANE_SPLIT_ACTIONS_HORIZONTAL_PADDING * 2 + PANE_SPLIT_ACTIONS_OUTER_MARGIN; const PANE_MAXIMIZE_ACTION_RESERVED_WIDTH = smallIconButtonChromeFrameSize(false) + 1; -// The terminal slot holds two icon buttons (upload, mic). While the terminal dictates the -// mic grows into a pill; the row re-measures then instead of reserving that width always. -const TERMINAL_SLOT_RESERVED_WIDTH = 2 * (smallIconButtonChromeFrameSize(false) + 1); +// The toolbar's gap between controls (ToolbarControls). +const TOOLBAR_CONTROL_GAP = 1; +// The terminal slot's width before its first layout: two icon buttons (upload, mic). The slot +// is measured afterwards, because the mic grows into a pill while the terminal dictates and the +// row itself keeps its width when that happens. +const TERMINAL_SLOT_IDLE_WIDTH = 2 * smallIconButtonChromeFrameSize(false) + TOOLBAR_CONTROL_GAP; // Chip geometry. `layoutMetrics` measures tabs from these same numbers, so a chip that changes // shape without changing them mis-measures and drops the row into the overflow-scroll fallback at // the wrong width. Keep them together. @@ -1093,17 +1096,24 @@ function ResolvedWorkspaceDesktopTabsRow({ updateMeasuredWidth(setExitFocusModeWidth, event); }, []); + const [terminalSlotWidth, setTerminalSlotWidth] = useState(TERMINAL_SLOT_IDLE_WIDTH); + const handleTerminalSlotLayout = useCallback((event: LayoutChangeEvent) => { + updateMeasuredWidth(setTerminalSlotWidth, event); + }, []); + const activeTerminalId = resolveSlotTerminalId(tabs); const terminalSlot = useMemo( () => activeTerminalId ? ( - + style={styles.terminalSlot} + onLayout={handleTerminalSlotLayout} + > + + ) : null, - [activeTerminalId, normalizedServerId], + [activeTerminalId, handleTerminalSlotLayout, normalizedServerId], ); const layoutMetrics = useMemo( @@ -1115,7 +1125,7 @@ function ResolvedWorkspaceDesktopTabsRow({ (focusModeEnabled ? exitFocusModeWidth : 0) + (showPaneSplitActions ? PANE_SPLIT_ACTIONS_RESERVED_WIDTH : 0) + (showPaneMaximizeAction ? PANE_MAXIMIZE_ACTION_RESERVED_WIDTH : 0) + - (activeTerminalId ? TERMINAL_SLOT_RESERVED_WIDTH : 0), + (activeTerminalId ? terminalSlotWidth + TOOLBAR_CONTROL_GAP : 0), ), rowPaddingHorizontal: TAB_ROW_PADDING_HORIZONTAL, tabGap: TAB_CHIP_GAP, @@ -1132,6 +1142,7 @@ function ResolvedWorkspaceDesktopTabsRow({ focusModeEnabled, showPaneMaximizeAction, showPaneSplitActions, + terminalSlotWidth, ], ); @@ -1688,6 +1699,11 @@ const styles = StyleSheet.create((theme) => ({ paddingHorizontal: PANE_SPLIT_ACTIONS_HORIZONTAL_PADDING, marginRight: PANE_SPLIT_ACTIONS_OUTER_MARGIN, }, + terminalSlot: { + flexDirection: "row", + alignItems: "center", + gap: TOOLBAR_CONTROL_GAP, + }, tab: { height: buttonControlHeight.xs, paddingHorizontal: TAB_CHIP_HORIZONTAL_PADDING,