Merge pull request #4299 from code-yeongyu/fix/issue-4128-ctx-dollar-guard

fix(session-notification-sender): guard ctx.$ availability with execFile fallback (#4128, #4061)

src/hooks/session-notification-sender.test.ts

@@ -1,4 +1,7 @@
+/// <reference types="bun-types" />
+
 import { afterEach, beforeEach, describe, expect, jest, spyOn, test } from "bun:test"
+import * as childProcess from "node:child_process"
 import * as sender from "./session-notification-sender"
 import * as utils from "./session-notification-utils"
 import type { PluginInput } from "@opencode-ai/plugin"
@@ -6,6 +9,9 @@ import { unsafeTestValue } from "../../test-support/unsafe-test-value"
 
 
 
+type TestShellResult = ReturnType<NonNullable<PluginInput["$"]>>
+type TestShellFactory = (cmd: TemplateStringsArray, ...values: unknown[]) => TestShellResult
+
 function createShellPromise(handler: (cmdStr: string) => void) {
 	return (cmd: TemplateStringsArray, ...values: unknown[]) => {
 		const cmdStr = cmd.reduce((acc, part, i) => acc + part + (values[i] ?? ""), "")
@@ -64,6 +70,29 @@ function createThrowingShellPromise(shouldThrow: (cmdStr: string) => boolean) {
 	}
 }
 
+type ExecFileCall = {
+	readonly file: string
+	readonly args: readonly string[]
+	readonly options: { readonly windowsHide?: boolean }
+}
+
+function mockExecFile(calls: ExecFileCall[], error: Error | null = null): ReturnType<typeof spyOn> {
+	return spyOn(childProcess, "execFile").mockImplementation(
+		unsafeTestValue<typeof childProcess.execFile>(
+			(
+				file: string,
+				args: readonly string[],
+				options: { readonly windowsHide?: boolean },
+				callback: (execError: Error | null, stdout: string, stderr: string) => void
+			) => {
+				calls.push({ file, args: [...args], options })
+				callback(error, "", "")
+				return unsafeTestValue<ReturnType<typeof childProcess.execFile>>({})
+			}
+		)
+	)
+}
+
 describe("session-notification-sender", () => {
 	beforeEach(() => {
 		jest.restoreAllMocks()
@@ -77,34 +106,33 @@ describe("session-notification-sender", () => {
 		spyOn(utils, "getAplayPath").mockResolvedValue("/usr/bin/aplay")
 	})
 
+	afterEach(() => {
+		jest.restoreAllMocks()
+	})
+
 	describe("#given sendSessionNotification", () => {
 		describe("#when ctx.$ is unavailable", () => {
-			test("#then it returns early without throwing when ctx has no $", async () => {
+			test("#then it falls back to execFile without throwing", async () => {
-				const cmuxSpy = spyOn(utils, "getCmuxPath")
+				const execFileCalls: ExecFileCall[] = []
+				mockExecFile(execFileCalls)
 				const mockCtx = unsafeTestValue<PluginInput>({})
 
-				await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined()
+				await sender.sendSessionNotification(mockCtx, "win32", "Test", "Message")
-				expect(cmuxSpy).not.toHaveBeenCalled()
+
+				expect(execFileCalls.length).toBe(1)
+				expect(execFileCalls[0]?.file).toBe("powershell")
+				expect(execFileCalls[0]?.args[0]).toBe("-Command")
+				expect(execFileCalls[0]?.options.windowsHide).toBe(true)
 			})
 
-			test("#then it returns early without throwing when ctx.$ is not a function", async () => {
+			test("#then it swallows execFile rejection without throwing", async () => {
-				const cmuxSpy = spyOn(utils, "getCmuxPath")
+				const execFileCalls: ExecFileCall[] = []
-				const mockCtx = unsafeTestValue<PluginInput>({
+				mockExecFile(execFileCalls, new Error("execFile failed"))
-					$: "not-a-function",
-				})
-
-				await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined()
-				expect(cmuxSpy).not.toHaveBeenCalled()
-			})
-
-			test("#then it remains non-throwing across sender APIs", async () => {
-				const afplaySpy = spyOn(utils, "getAfplayPath")
 				const mockCtx = unsafeTestValue<PluginInput>({})
 
-				await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined()
+				await sender.sendSessionNotification(mockCtx, "win32", "Test", "Message")
-				await expect(sender.playSessionNotificationSound(mockCtx, "darwin", "/sound.aiff")).resolves.toBeUndefined()
 
-				expect(afplaySpy).not.toHaveBeenCalled()
+				expect(execFileCalls.length).toBe(1)
 			})
 		})
 
@@ -192,13 +220,13 @@ describe("session-notification-sender", () => {
 					$: createThrowingShellPromise((cmdStr) => cmdStr.includes("cmux notify")),
 				})
 
-				const originalFactory = mockCtx.$
+				const originalFactory = unsafeTestValue<TestShellFactory>(mockCtx.$)
 				const trackingCalls: string[] = []
-				mockCtx.$ = ((cmd: TemplateStringsArray, ...values: unknown[]) => {
+				mockCtx.$ = unsafeTestValue<typeof mockCtx.$>((cmd: TemplateStringsArray, ...values: unknown[]) => {
 					const cmdStr = cmd.reduce((acc: string, part: string, i: number) => acc + part + (values[i] ?? ""), "")
 					trackingCalls.push(cmdStr)
 					return originalFactory(cmd, ...values)
-				}) as typeof mockCtx.$
+				})
 
 				await sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")
 
@@ -215,12 +243,12 @@ describe("session-notification-sender", () => {
 					$: createThrowingShellPromise((cmdStr) => cmdStr.includes("cmux notify") || cmdStr.includes("terminal-notifier")),
 				})
 
-				const originalFactory = mockCtx.$
+				const originalFactory = unsafeTestValue<TestShellFactory>(mockCtx.$)
-				mockCtx.$ = ((cmd: TemplateStringsArray, ...values: unknown[]) => {
+				mockCtx.$ = unsafeTestValue<typeof mockCtx.$>((cmd: TemplateStringsArray, ...values: unknown[]) => {
 					const cmdStr = cmd.reduce((acc: string, part: string, i: number) => acc + part + (values[i] ?? ""), "")
 					trackingCalls.push(cmdStr)
 					return originalFactory(cmd, ...values)
-				}) as typeof mockCtx.$
+				})
 
 				await sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")
 

src/hooks/session-notification-sender.ts

@@ -1,4 +1,6 @@
 import type { PluginInput } from "@opencode-ai/plugin"
+import { execFile } from "node:child_process"
+import { promisify } from "node:util"
 import { platform } from "os"
 import { log } from "../shared"
 import {
@@ -39,17 +41,45 @@ type ShellCommand = Promise<unknown> & {
   nothrow?: () => ShellCommand
 }
 
+type ShellRunner = NonNullable<PluginInput["$"]>
+
+type ShellFailureMode = "throw" | "nothrow"
+
 let hasLoggedUnavailableShellHelper = false
 
-function canRunNotificationCommand(ctx: PluginInput): boolean {
+function getShellRunner(ctx: PluginInput): ShellRunner | undefined {
-  if (typeof ctx?.$ === "function") return true
+  // Guard for #4128 + #4061: OpenCode Desktop's Electron sidecar can omit Bun's ctx.$ helper.
+  if (typeof ctx.$ === "function") return ctx.$
 
   if (!hasLoggedUnavailableShellHelper) {
     hasLoggedUnavailableShellHelper = true
-    log("[session-notification] ctx.$ unavailable; skipping notification command execution")
+    log("[session-notification] ctx.$ unavailable; falling back to child_process.execFile")
   }
 
-  return false
+  return undefined
+}
+
+function logCommandFailure(commandName: string, error: Error | string): void {
+  log("[session-notification] notification command failed", {
+    commandName,
+    error: typeof error === "string" ? error : error.message,
+  })
+}
+
+function logOperationFailure(operation: string, error: Error | string): void {
+  log("[session-notification] notification operation failed", {
+    operation,
+    error: typeof error === "string" ? error : error.message,
+  })
+}
+
+async function runQuiet(command: ShellCommand): Promise<void> {
+  if (typeof command.quiet === "function") {
+    await command.quiet()
+    return
+  }
+
+  await command
 }
 
 async function runQuietNothrow(command: ShellCommand): Promise<void> {
@@ -62,64 +92,135 @@ async function runQuietNothrow(command: ShellCommand): Promise<void> {
   await safeCommand
 }
 
+async function runExecFile(commandPath: string, args: readonly string[]): Promise<void> {
+  const execFileAsync = promisify(execFile)
+  await execFileAsync(commandPath, [...args], { windowsHide: true })
+}
+
+async function runNotificationCommand(
+  ctx: PluginInput,
+  commandPath: string,
+  args: readonly string[],
+  shellCommand: (shell: ShellRunner) => ShellCommand,
+  shellFailureMode: ShellFailureMode = "nothrow"
+): Promise<void> {
+  const shell = getShellRunner(ctx)
+  if (shell) {
+    if (shellFailureMode === "throw") {
+      await runQuiet(shellCommand(shell))
+      return
+    }
+
+    await runQuietNothrow(shellCommand(shell))
+    return
+  }
+
+  await runExecFile(commandPath, args)
+}
+
 export async function sendSessionNotification(
   ctx: PluginInput,
   platform: Platform,
   title: string,
   message: string
 ): Promise<void> {
-  if (!canRunNotificationCommand(ctx)) return
+  try {
-
+    switch (platform) {
-  switch (platform) {
+      case "darwin": {
-    case "darwin": {
+        // Try cmux first - native UNUserNotificationCenter, properly attributed
-      // Try cmux first - native UNUserNotificationCenter, properly attributed
+        const cmuxPath = await getCmuxPath()
-      const cmuxPath = await getCmuxPath()
+        if (cmuxPath) {
-      if (cmuxPath) {
+          try {
-        try {
+            await runNotificationCommand(
-          await ctx.$`${cmuxPath} notify --title ${title} --body ${message}`.quiet()
+              ctx,
-          break
+              cmuxPath,
-        } catch {
+              ["notify", "--title", title, "--body", message],
-        }
+              (shell) => shell`${cmuxPath} notify --title ${title} --body ${message}`,
-      }
+              "throw"
-
+            )
-      // Try terminal-notifier - deterministic click-to-focus
+            break
-      const terminalNotifierPath = await getTerminalNotifierPath()
+          } catch (error) {
-      if (terminalNotifierPath) {
+            if (error instanceof Error) {
-        const bundleId = process.env.__CFBundleIdentifier
+              logCommandFailure("cmux", error)
-        try {
+            } else {
-          if (bundleId) {
+              logCommandFailure("cmux", String(error))
-            await ctx.$`${terminalNotifierPath} -title ${title} -message ${message} -activate ${bundleId}`.quiet()
+            }
-          } else {
-            await ctx.$`${terminalNotifierPath} -title ${title} -message ${message}`.quiet()
           }
-          break
-        } catch {
         }
+
+        // Try terminal-notifier - deterministic click-to-focus
+        const terminalNotifierPath = await getTerminalNotifierPath()
+        if (terminalNotifierPath) {
+          const bundleId = process.env.__CFBundleIdentifier
+          const args = bundleId
+            ? ["-title", title, "-message", message, "-activate", bundleId]
+            : ["-title", title, "-message", message]
+          try {
+            await runNotificationCommand(
+              ctx,
+              terminalNotifierPath,
+              args,
+              (shell) => bundleId
+                ? shell`${terminalNotifierPath} -title ${title} -message ${message} -activate ${bundleId}`
+                : shell`${terminalNotifierPath} -title ${title} -message ${message}`,
+              "throw"
+            )
+            break
+          } catch (error) {
+            if (error instanceof Error) {
+              logCommandFailure("terminal-notifier", error)
+            } else {
+              logCommandFailure("terminal-notifier", String(error))
+            }
+          }
+        }
+
+        // Fallback: osascript (click may open Finder instead of terminal)
+        const osascriptPath = await getOsascriptPath()
+        if (!osascriptPath) return
+
+        const escapedTitle = escapeAppleScriptText(title)
+        const escapedMessage = escapeAppleScriptText(message)
+        const appleScript = "display notification \"" + escapedMessage + "\" with title \"" + escapedTitle + "\""
+        await runNotificationCommand(
+          ctx,
+          osascriptPath,
+          ["-e", appleScript],
+          (shell) => shell`${osascriptPath} -e ${appleScript}`
+        )
+        break
       }
+      case "linux": {
+        const notifySendPath = await getNotifySendPath()
+        if (!notifySendPath) return
 
-      // Fallback: osascript (click may open Finder instead of terminal)
+        await runNotificationCommand(
-      const osascriptPath = await getOsascriptPath()
+          ctx,
-      if (!osascriptPath) return
+          notifySendPath,
+          [title, message],
+          (shell) => shell`${notifySendPath} ${title} ${message} 2>/dev/null`
+        )
+        break
+      }
+      case "win32": {
+        const powershellPath = await getPowershellPath()
+        if (!powershellPath) return
 
-      const escapedTitle = escapeAppleScriptText(title)
+        const toastScript = buildWindowsToastScript(title, message)
-      const escapedMessage = escapeAppleScriptText(message)
+        await runNotificationCommand(
-      await runQuietNothrow(ctx.$`${osascriptPath} -e ${"display notification \"" + escapedMessage + "\" with title \"" + escapedTitle + "\""}`)
+          ctx,
-      break
+          powershellPath,
+          ["-Command", toastScript],
+          (shell) => shell`${powershellPath} -Command ${toastScript}`
+        )
+        break
+      }
     }
-    case "linux": {
+  } catch (error) {
-      const notifySendPath = await getNotifySendPath()
+    if (error instanceof Error) {
-      if (!notifySendPath) return
+      logOperationFailure("send", error)
-
+    } else {
-      await runQuietNothrow(ctx.$`${notifySendPath} ${title} ${message} 2>/dev/null`)
+      logOperationFailure("send", String(error))
-      break
-    }
-    case "win32": {
-      const powershellPath = await getPowershellPath()
-      if (!powershellPath) return
-
-      const toastScript = buildWindowsToastScript(title, message)
-      await runQuietNothrow(ctx.$`${powershellPath} -Command ${toastScript}`)
-      break
     }
   }
 }
@@ -129,33 +230,60 @@ export async function playSessionNotificationSound(
   platform: Platform,
   soundPath: string
 ): Promise<void> {
-  if (!canRunNotificationCommand(ctx)) return
+  try {
-
+    switch (platform) {
-  switch (platform) {
+      case "darwin": {
-    case "darwin": {
+        const afplayPath = await getAfplayPath()
-      const afplayPath = await getAfplayPath()
+        if (!afplayPath) return
-      if (!afplayPath) return
+        await runNotificationCommand(
-      await runQuietNothrow(ctx.$`${afplayPath} ${soundPath}`)
+          ctx,
-      break
+          afplayPath,
-    }
+          [soundPath],
-    case "linux": {
+          (shell) => shell`${afplayPath} ${soundPath}`
-      const paplayPath = await getPaplayPath()
+        )
-      if (paplayPath) {
+        break
-        await runQuietNothrow(ctx.$`${paplayPath} ${soundPath} 2>/dev/null`)
+      }
-      } else {
+      case "linux": {
-        const aplayPath = await getAplayPath()
+        const paplayPath = await getPaplayPath()
-        if (aplayPath) {
+        if (paplayPath) {
-          await runQuietNothrow(ctx.$`${aplayPath} ${soundPath} 2>/dev/null`)
+          await runNotificationCommand(
-        }
+            ctx,
+            paplayPath,
+            [soundPath],
+            (shell) => shell`${paplayPath} ${soundPath} 2>/dev/null`
+          )
+        } else {
+          const aplayPath = await getAplayPath()
+          if (aplayPath) {
+            await runNotificationCommand(
+              ctx,
+              aplayPath,
+              [soundPath],
+              (shell) => shell`${aplayPath} ${soundPath} 2>/dev/null`
+            )
+          }
+        }
+        break
+      }
+      case "win32": {
+        const powershellPath = await getPowershellPath()
+        if (!powershellPath) return
+        const escaped = escapePowerShellSingleQuotedText(soundPath)
+        const soundScript = "(New-Object Media.SoundPlayer '" + escaped + "').PlaySync()"
+        await runNotificationCommand(
+          ctx,
+          powershellPath,
+          ["-Command", soundScript],
+          (shell) => shell`${powershellPath} -Command ${soundScript}`
+        )
+        break
       }
-      break
     }
-    case "win32": {
+  } catch (error) {
-      const powershellPath = await getPowershellPath()
+    if (error instanceof Error) {
-      if (!powershellPath) return
+      logOperationFailure("sound", error)
-      const escaped = escapePowerShellSingleQuotedText(soundPath)
+    } else {
-      await runQuietNothrow(ctx.$`${powershellPath} -Command ${"(New-Object Media.SoundPlayer '" + escaped + "').PlaySync()"}`)
+      logOperationFailure("sound", String(error))
-      break
     }
   }
 }