Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions apps/desktop/src/main/managers/recording-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,13 @@ export class RecordingManager extends EventEmitter {
const preferences = await settingsService.getPreferences();
const shouldMute = preferences?.muteSystemAudio ?? true;

// Always play rec-start sound, await completion before muting (so chime isn't cut off)
try {
await nativeBridge.call("playSound", { sound: "rec-start" });
} catch (error) {
logger.audio.warn("Failed to play rec-start sound", { error });
}
Comment on lines +304 to +309
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Handle playSound business failures (success: false) explicitly.

At Line [306] and Line [384], the call can resolve successfully while playback still fails (success: false). Today only rejected promises are logged, so silent playback failures are missed.

Suggested patch
-      try {
-        await nativeBridge.call("playSound", { sound: "rec-start" });
+      try {
+        const startSound = await nativeBridge.call("playSound", {
+          sound: "rec-start",
+        });
+        if (!startSound.success) {
+          logger.audio.warn("rec-start playback reported failure", {
+            message: startSound.message,
+          });
+        }
       } catch (error) {
         logger.audio.warn("Failed to play rec-start sound", { error });
       }
@@
       this.serviceManager
         .getService("nativeBridge")
         .call("playSound", { sound: "rec-stop" })
+        .then((stopSound) => {
+          if (!stopSound.success) {
+            logger.audio.warn("rec-stop playback reported failure", {
+              message: stopSound.message,
+            });
+          }
+        })
         .catch((error) => {
           logger.audio.warn("Failed to play rec-stop sound", { error });
         });

Also applies to: 381-387

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/main/managers/recording-manager.ts` around lines 304 - 309,
The nativeBridge.call("playSound", { sound: "rec-start" }) invocation currently
only catches rejected promises but ignores resolved responses where playback
failed (response.success === false); update the await calls in RecordingManager
(the rec-start playback and the later playSound usage) to inspect the returned
value, and if the response exists and has success === false, log a warning via
logger.audio.warn (including the response/error details) and treat it as a
failure path (e.g., do not proceed to mute or continue as if sound played);
ensure both call sites that invoke playSound check the response.success flag and
handle/log accordingly.


if (shouldMute) {
const result = await nativeBridge.call("muteSystemAudio", {});
this.systemAudioMuted = !!result?.success;
Expand Down Expand Up @@ -371,6 +378,14 @@ export class RecordingManager extends EventEmitter {
logger.main.warn("Failed to restore system audio", { error });
}

// Always play rec-stop sound (fire-and-forget)
this.serviceManager
.getService("nativeBridge")
.call("playSound", { sound: "rec-stop" })
.catch((error) => {
logger.audio.warn("Failed to play rec-stop sound", { error });
});

// Cancel streaming for cancel codes (not null, not dismissed)
if (code && code !== "dismissed" && sessionId) {
try {
Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/src/services/platform/native-bridge-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ import {
RestoreSystemAudioParams,
RestoreSystemAudioResult,
RestoreSystemAudioResultSchema,
PlaySoundParams,
PlaySoundResult,
PlaySoundResultSchema,
SetShortcutsParams,
SetShortcutsResult,
SetShortcutsResultSchema,
Expand Down Expand Up @@ -77,6 +80,10 @@ interface RPCMethods {
params: RestoreSystemAudioParams;
result: RestoreSystemAudioResult;
};
playSound: {
params: PlaySoundParams;
result: PlaySoundResult;
};
setShortcuts: {
params: SetShortcutsParams;
result: SetShortcutsResult;
Expand Down Expand Up @@ -106,6 +113,7 @@ const RPC_RESULT_SCHEMAS: Record<keyof RPCMethods, ZodTypeAny> = {
pasteText: PasteTextResultSchema,
muteSystemAudio: MuteSystemAudioResultSchema,
restoreSystemAudio: RestoreSystemAudioResultSchema,
playSound: PlaySoundResultSchema,
setShortcuts: SetShortcutsResultSchema,
recheckPressedKeys: RecheckPressedKeysResultSchema,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Foundation

class AudioService: NSObject, AVAudioPlayerDelegate {
private var audioPlayer: AVAudioPlayer?
private var audioCompletionHandler: (() -> Void)?
private var audioCompletionHandler: ((Bool) -> Void)?
private var preloadedAudio: [String: Data] = [:]
override init() {
super.init()
Expand All @@ -21,10 +21,10 @@ class AudioService: NSObject, AVAudioPlayerDelegate {
logToStderr("[AudioService] Audio files preloaded at startup")
}

func playSound(named soundName: String, completion: (() -> Void)? = nil) {
func playSound(named soundName: String, completion: ((Bool) -> Void)? = nil) {
logToStderr("[AudioService] playSound called with soundName: \(soundName)")

// Stop any currently playing sound
// Stop any currently playing sound and complete the previous handler as interrupted
if audioPlayer?.isPlaying == true {
logToStderr(
"[AudioService] Sound '\(audioPlayer?.url?.lastPathComponent ?? "previous")' is playing. Stopping it."
Expand All @@ -33,7 +33,9 @@ class AudioService: NSObject, AVAudioPlayerDelegate {
audioPlayer?.stop()
}
audioPlayer = nil
let previousHandler = audioCompletionHandler
audioCompletionHandler = nil
previousHandler?(false)

audioCompletionHandler = completion

Expand All @@ -50,8 +52,10 @@ class AudioService: NSObject, AVAudioPlayerDelegate {
case "rec-stop":
soundData = Data(PackageResources.rec_stop_mp3)
default:
logToStderr("[AudioService] Error: Unknown sound name '\(soundName)'. Completion will not be called.")
logToStderr("[AudioService] Error: Unknown sound name '\(soundName)'. Calling completion immediately.")
let handler = audioCompletionHandler
audioCompletionHandler = nil
handler?(false)
return
}
}
Expand All @@ -64,15 +68,19 @@ class AudioService: NSObject, AVAudioPlayerDelegate {
logToStderr("[AudioService] Playing sound: \(soundName).mp3. Delegate will handle completion.")
} else {
logToStderr(
"[AudioService] Failed to start playing sound: \(soundName).mp3. Completion will not be called."
"[AudioService] Failed to start playing sound: \(soundName).mp3. Calling completion immediately."
)
let handler = audioCompletionHandler
audioCompletionHandler = nil
handler?(false)
}
} catch {
logToStderr(
"[AudioService] Error initializing AVAudioPlayer for \(soundName).mp3: \(error.localizedDescription). Completion will not be called."
"[AudioService] Error initializing AVAudioPlayer for \(soundName).mp3: \(error.localizedDescription). Calling completion immediately."
)
let handler = audioCompletionHandler
audioCompletionHandler = nil
handler?(false)
}
}

Expand All @@ -88,10 +96,10 @@ class AudioService: NSObject, AVAudioPlayerDelegate {

if flag {
logToStderr("[AudioService] Sound finished successfully. Executing completion handler.")
handlerToCall?()
} else {
logToStderr("[AudioService] Sound did not finish successfully. Not executing completion handler.")
logToStderr("[AudioService] Sound did not finish successfully. Executing completion handler anyway.")
}
handlerToCall?(flag)
}

private func logToStderr(_ message: String) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,50 +95,29 @@ class IOBridge: NSObject {
case .muteSystemAudio:
logToStderr("[IOBridge] Handling muteSystemAudio for ID: \(request.id)")

audioService.playSound(named: "rec-start") { [weak self] in
guard let self = self else {
HelperLogger.logToStderr(
"[IOBridge] self is nil in playSound completion for muteSystemAudio. ID: \(request.id)"
)
return
}
let muteSuccess = accessibilityService.muteSystemAudio()
let muteResultPayload = MuteSystemAudioResultSchema(
message: muteSuccess ? "Mute command sent" : "Failed to send mute command",
success: muteSuccess)

self.logToStderr(
"[IOBridge] rec-start.mp3 finished playing successfully. Proceeding to mute system audio. ID: \(request.id)"
do {
let resultData = try jsonEncoder.encode(muteResultPayload)
let resultAsJsonAny = try jsonDecoder.decode(JSONAny.self, from: resultData)
rpcResponse = RPCResponseSchema(error: nil, id: request.id, result: resultAsJsonAny)
} catch {
logToStderr(
"[IOBridge] Error encoding muteSystemAudio result: \(error.localizedDescription) for ID: \(request.id)"
)
let success = self.accessibilityService.muteSystemAudio()
let resultPayload = MuteSystemAudioResultSchema(
message: success ? "Mute command sent" : "Failed to send mute command",
success: success)

var responseToSend: RPCResponseSchema
do {
let resultData = try self.jsonEncoder.encode(resultPayload)
let resultAsJsonAny = try self.jsonDecoder.decode(
JSONAny.self, from: resultData)
responseToSend = RPCResponseSchema(
error: nil, id: request.id, result: resultAsJsonAny)
} catch {
self.logToStderr(
"[IOBridge] Error encoding muteSystemAudio result: \(error.localizedDescription) for ID: \(request.id)"
)
let errPayload = Error(
code: -32603, data: nil,
message: "Error encoding result: \(error.localizedDescription)")
responseToSend = RPCResponseSchema(
error: errPayload, id: request.id, result: nil)
}
self.sendRpcResponse(responseToSend)
let errPayload = Error(
code: -32603, data: nil,
message: "Error encoding result: \(error.localizedDescription)")
rpcResponse = RPCResponseSchema(error: errPayload, id: request.id, result: nil)
}
return

case .restoreSystemAudio:
logToStderr("[IOBridge] Handling restoreSystemAudio for ID: \(request.id)")

let success = accessibilityService.restoreSystemAudio()
if success { // Play sound only if restore was successful
audioService.playSound(named: "rec-stop")
}
let resultPayload = RestoreSystemAudioResultSchema(
message: success ? "Restore command sent" : "Failed to send restore command",
success: success)
Expand Down Expand Up @@ -230,6 +209,44 @@ class IOBridge: NSObject {
rpcResponse = RPCResponseSchema(error: errPayload, id: request.id, result: nil)
}

case .playSound:
logToStderr("[IOBridge] Handling playSound for ID: \(request.id)")
guard let paramsAnyCodable = request.params else {
let errPayload = Error(
code: -32602, data: nil, message: "Missing params for playSound")
rpcResponse = RPCResponseSchema(error: errPayload, id: request.id, result: nil)
sendRpcResponse(rpcResponse)
return
}

do {
let paramsData = try jsonEncoder.encode(paramsAnyCodable)
let playSoundParams = try jsonDecoder.decode(
PlaySoundParamsSchema.self, from: paramsData)

audioService.playSound(named: playSoundParams.sound) { [weak self] success in
guard let self = self else {
HelperLogger.logToStderr(
"[IOBridge] self is nil in playSound completion. ID: \(request.id)")
return
}
Comment on lines +227 to +232
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not drop the RPC response when self is nil in playSound completion.

At Line [228], the nil-self branch returns without sendResult/sendError, so the request can hang until timeout.

Suggested patch
-                audioService.playSound(named: playSoundParams.sound) { [weak self] success in
-                    guard let self = self else {
-                        HelperLogger.logToStderr(
-                            "[IOBridge] self is nil in playSound completion. ID: \(request.id)")
-                        return
-                    }
-                    self.sendResult(
+                audioService.playSound(named: playSoundParams.sound) { success in
+                    self.sendResult(
                         id: request.id,
                         result: PlaySoundResultSchema(
                             message: success ? "Sound playback completed" : "Sound playback failed",
                             success: success))
                 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
audioService.playSound(named: playSoundParams.sound) { [weak self] success in
guard let self = self else {
HelperLogger.logToStderr(
"[IOBridge] self is nil in playSound completion. ID: \(request.id)")
return
}
audioService.playSound(named: playSoundParams.sound) { success in
self.sendResult(
id: request.id,
result: PlaySoundResultSchema(
message: success ? "Sound playback completed" : "Sound playback failed",
success: success))
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/native-helpers/swift-helper/Sources/SwiftHelper/RpcHandler.swift`
around lines 227 - 232, The completion handler for audioService.playSound in
RpcHandler.swift currently returns early when self is nil and thereby never
responds to the RPC; modify the nil-self branch to send a failure RPC response
(e.g., call sendError or sendResult with an error) using the request.id so the
caller is notified instead of hanging. Locate the closure passed to
audioService.playSound (the block capturing [weak self]) and ensure that when
guard let self fails you call the RpcHandler's response helper
(sendError/sendResult) with a descriptive message including request.id before
returning.

self.sendResult(
id: request.id,
result: PlaySoundResultSchema(
message: success ? "Sound playback completed" : "Sound playback failed",
success: success))
}
return
} catch {
logToStderr(
"[IOBridge] Error processing playSound params: \(error.localizedDescription) for ID: \(request.id)"
)
let errPayload = Error(
code: -32602, data: request.params,
message: "Invalid params: \(error.localizedDescription)")
rpcResponse = RPCResponseSchema(error: errPayload, id: request.id, result: nil)
}

default:
logToStderr("[IOBridge] Method not found: \(request.method) for ID: \(request.id)")
let errPayload = Error(
Expand Down
1 change: 1 addition & 0 deletions packages/types/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export * from "./schemas/methods/get-accessibility-context.js";
export * from "./schemas/methods/paste-text.js";
export * from "./schemas/methods/mute-system-audio.js";
export * from "./schemas/methods/restore-system-audio.js";
export * from "./schemas/methods/play-sound.js";
export * from "./schemas/methods/set-shortcuts.js";
export * from "./schemas/methods/recheck-pressed-keys.js";

Expand Down
14 changes: 14 additions & 0 deletions packages/types/src/schemas/methods/play-sound.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { z } from "zod";

// Request params
export const PlaySoundParamsSchema = z.object({
sound: z.enum(["rec-start", "rec-stop"]),
});
export type PlaySoundParams = z.infer<typeof PlaySoundParamsSchema>;

// Response result
export const PlaySoundResultSchema = z.object({
success: z.boolean(),
message: z.string().optional(),
});
export type PlaySoundResult = z.infer<typeof PlaySoundResultSchema>;
1 change: 1 addition & 0 deletions packages/types/src/schemas/rpc/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const RPCMethodNameSchema = z.union([
z.literal("pasteText"),
z.literal("muteSystemAudio"),
z.literal("restoreSystemAudio"),
z.literal("playSound"),
z.literal("setShortcuts"),
z.literal("recheckPressedKeys"),
]);
Expand Down
Loading