diff --git a/OSGKeyboard/Views/KeyboardPreviewSheet.swift b/OSGKeyboard/Views/KeyboardPreviewSheet.swift index 24086b9..933bed0 100644 --- a/OSGKeyboard/Views/KeyboardPreviewSheet.swift +++ b/OSGKeyboard/Views/KeyboardPreviewSheet.swift @@ -101,6 +101,16 @@ struct KeyboardPreviewSheet: View { insertRecognizedText(new) asr.reset() } + // Tear down the ASR pipeline when the sheet leaves the screen + // (preview dismissed, app backgrounded mid-recording, etc.). + // Without this, a leftover `asrTask` keeps the AVAudioSession + // active and the mic permission in use after the user has + // moved on. `asr.stop()` is idempotent — it no-ops on + // `.idle`/`.denied`/`.error` — so it's safe to call here + // even when the disc is not currently recording. + .onDisappear { + asr.stop() + } } /// Real `TextField` (was a static placeholder HStack before the diff --git a/OSGKeyboard/Views/PreviewASRController.swift b/OSGKeyboard/Views/PreviewASRController.swift index cd1bc74..a237ed3 100644 --- a/OSGKeyboard/Views/PreviewASRController.swift +++ b/OSGKeyboard/Views/PreviewASRController.swift @@ -50,7 +50,13 @@ final class PreviewASRController: ObservableObject { private let asr: ASRService = ASRServiceFactory.make() private let audioEngine = AVAudioEngine() - private var asrTask: Task? + /// `internal` (not `private`) so the regression test in + /// `OSGKeyboardTests/PreviewASRControllerStateTests.swift` can + /// install a known consumer task and assert `stop()` doesn't + /// cancel it. The class is `@MainActor`-isolated, so the + /// natural Swift 6 isolation rules still prevent production + /// code outside the class from racing on it. + var asrTask: Task? private var bufferContinuation: AsyncStream.Continuation? private var didConfigureAudioSession = false private var didInstallTap = false @@ -65,6 +71,15 @@ final class PreviewASRController: ObservableObject { default: break } + // Cancel any leftover consumer task from a previous recording. + // Normally `stop()` lets the task run to completion (so it can + // see the `.final` and transition out of `.processing`), but if + // the user smashed the disc twice — stop, then immediately + // start — the previous task might still be draining. Cancel it + // here so we don't have two consumer tasks fighting over the + // same `events` stream. + asrTask?.cancel() + asrTask = nil phase = .requestingPermission currentPartial = "" lastFinal = "" @@ -111,8 +126,20 @@ final class PreviewASRController: ObservableObject { } func stop() { - asrTask?.cancel() - asrTask = nil + // Don't `asrTask?.cancel()` here — see the comment in + // `startEngineAndASR` for the full rationale. Short version: + // cancelling the consumer task at the same moment we close the + // audio stream also triggers the producer's + // `continuation.onTermination → self?.cancel()` cascade, which + // marks the producer's outer task as cancelled and skips the + // `.final` event. The UI is then left in `.processing` forever + // because no one schedules the transition out. The consumer + // task naturally exits when `events` finishes, so the right + // thing is to let it run. + // + // If a previous `asrTask` is somehow still running (e.g. the + // user smashed the disc twice quickly), `start()` cancels it + // at the entry point as a safety net. if didInstallTap { audioEngine.inputNode.removeTap(onBus: 0) didInstallTap = false @@ -128,6 +155,21 @@ final class PreviewASRController: ObservableObject { // Deactivate so the user's music resumes if the preview is // dismissed mid-recording. try? AVAudioSession.sharedInstance().setActive(false, options: .notifyOthersOnDeactivation) + + // Safety net: if the ASR pipeline never produces a `.final` + // (analyzer hang, system glitch, dropped continuation), force + // the UI back to idle after a short delay so the user isn't + // stuck. Normal recordings complete well under a second, so + // the 3-second budget is only hit on the unhappy path; if the + // pipeline finishes first and flips the phase to `.idle` (or + // `.error`), the check below no-ops. + Task { @MainActor [weak self] in + try? await Task.sleep(for: .seconds(3)) + guard let self else { return } + if self.phase == .processing { + self.phase = .idle + } + } } func reset() { diff --git a/OSGKeyboardTests/PreviewASRControllerStateTests.swift b/OSGKeyboardTests/PreviewASRControllerStateTests.swift new file mode 100644 index 0000000..d62ade2 --- /dev/null +++ b/OSGKeyboardTests/PreviewASRControllerStateTests.swift @@ -0,0 +1,70 @@ +// PreviewASRControllerStateTests.swift +// OSGKeyboard · Tests +// +// Locks in the state-machine contract for the keyboard preview's +// ASR controller. The previous bug was: `stop()` called +// `asrTask?.cancel()` on the consumer task that was waiting for the +// ASR `.final` event. The cancellation cascaded through +// `continuation.onTermination → self.cancel()` and suppressed the +// `.final` yield, leaving the controller in `.processing` forever +// because no one scheduled the transition out. The fix is: don't +// cancel on stop, let the consumer naturally see the `.final` and +// flip the phase. +// +// These tests don't drive a real ASR pipeline — they verify the +// state-machine contract directly. The class is `@MainActor` +// isolated, so all assertions happen on main. `asrTask` is +// `internal` (not `private`) precisely so this file can install a +// known consumer task and observe whether `stop()` cancels it. +// +// Note on the 3-second processing-timeout safety net: that +// fallback (a `Task { sleep 3s; if .processing → .idle }` block +// inside `stop()`) is not unit-tested here — testing it would +// require either a 3-second test or extracting the policy to a +// testable helper. The primary fix above removes the original +// bug outright; the timeout is a defensive safety net for a +// separate class of failure (analyzer hang) and is straightforward +// enough to read that a test adds little. If a future change +// touches the safety-net block, re-introduce a test that mocks +// the ASR to never yield `.final` and asserts the timeout fires. + +import XCTest +@testable import OSGKeyboard +import AVFoundation + +@MainActor +final class PreviewASRControllerStateTests: XCTestCase { + + /// Core fix: `stop()` MUST NOT cancel `asrTask`. The consumer + /// task is the only place the `.final` event can land, and + /// cancelling it leaves the UI in `.processing` forever. This + /// is the regression test for the original bug — if it ever + /// fails, the disc is stuck again. + func testStopDoesNotCancelConsumerTask() { + let controller = PreviewASRController() + let consumerTask = Task {} + controller.asrTask = consumerTask + + controller.stop() + + XCTAssertFalse( + consumerTask.isCancelled, + "stop() must not cancel the consumer task; the .final event needs the consumer alive to land" + ) + } + + /// Idempotency: calling `stop()` twice (e.g. `.onDisappear` + /// fires after the user already stopped manually) must not + /// misbehave — no double-cancel, no extra state transitions. + /// This is the contract `.onDisappear` relies on. + func testStopIsIdempotent() { + let controller = PreviewASRController() + let consumerTask = Task {} + controller.asrTask = consumerTask + + controller.stop() + controller.stop() + + XCTAssertFalse(consumerTask.isCancelled) + } +}