fix(session-notification-sender): guard ctx.$ with execFile fallback for Desktop sidecar (#4128, #4061)
OpenCode Desktop's Electron sidecar runtime can omit Bun's ctx.$ helper. The sender previously called ctx.$ unconditionally, throwing TypeError: ctx.$ is not a function as unhandledRejection and crashing the sidecar with exit code 1. Add a runtime guard at every call site, falling back to Node.js child_process.execFile (with windowsHide: true) when ctx.$ is missing. The Bun ctx.$ path remains preferred when available. Every notification path is wrapped in try/catch so no failure escapes as unhandledRejection. Fixes #4128 Fixes #4061
This commit is contained in:
YeonGyu-Kim
@@ -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 () => {
|
||||
const cmuxSpy = spyOn(utils, "getCmuxPath")
|
||||
test("#then it falls back to execFile without throwing", async () => {
|
||||
const execFileCalls: ExecFileCall[] = []
|
||||
mockExecFile(execFileCalls)
|
||||
const mockCtx = unsafeTestValue<PluginInput>({})
|
||||
|
||||
await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined()
|
||||
expect(cmuxSpy).not.toHaveBeenCalled()
|
||||
await sender.sendSessionNotification(mockCtx, "win32", "Test", "Message")
|
||||
|
||||
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 () => {
|
||||
const cmuxSpy = spyOn(utils, "getCmuxPath")
|
||||
const mockCtx = unsafeTestValue<PluginInput>({
|
||||
$: "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")
|
||||
test("#then it swallows execFile rejection without throwing", async () => {
|
||||
const execFileCalls: ExecFileCall[] = []
|
||||
mockExecFile(execFileCalls, new Error("execFile failed"))
|
||||
const mockCtx = unsafeTestValue<PluginInput>({})
|
||||
|
||||
await expect(sender.sendSessionNotification(mockCtx, "darwin", "Test", "Message")).resolves.toBeUndefined()
|
||||
await expect(sender.playSessionNotificationSound(mockCtx, "darwin", "/sound.aiff")).resolves.toBeUndefined()
|
||||
await sender.sendSessionNotification(mockCtx, "win32", "Test", "Message")
|
||||
|
||||
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.$
|
||||
mockCtx.$ = ((cmd: TemplateStringsArray, ...values: unknown[]) => {
|
||||
const originalFactory = unsafeTestValue<TestShellFactory>(mockCtx.$)
|
||||
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")
|
||||
|
||||
|
||||
@@ -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 {
|
||||
if (typeof ctx?.$ === "function") return true
|
||||
function getShellRunner(ctx: PluginInput): ShellRunner | undefined {
|
||||
// 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
|
||||
|
||||
switch (platform) {
|
||||
case "darwin": {
|
||||
// Try cmux first - native UNUserNotificationCenter, properly attributed
|
||||
const cmuxPath = await getCmuxPath()
|
||||
if (cmuxPath) {
|
||||
try {
|
||||
await ctx.$`${cmuxPath} notify --title ${title} --body ${message}`.quiet()
|
||||
break
|
||||
} catch {
|
||||
}
|
||||
}
|
||||
|
||||
// Try terminal-notifier - deterministic click-to-focus
|
||||
const terminalNotifierPath = await getTerminalNotifierPath()
|
||||
if (terminalNotifierPath) {
|
||||
const bundleId = process.env.__CFBundleIdentifier
|
||||
try {
|
||||
if (bundleId) {
|
||||
await ctx.$`${terminalNotifierPath} -title ${title} -message ${message} -activate ${bundleId}`.quiet()
|
||||
} else {
|
||||
await ctx.$`${terminalNotifierPath} -title ${title} -message ${message}`.quiet()
|
||||
try {
|
||||
switch (platform) {
|
||||
case "darwin": {
|
||||
// Try cmux first - native UNUserNotificationCenter, properly attributed
|
||||
const cmuxPath = await getCmuxPath()
|
||||
if (cmuxPath) {
|
||||
try {
|
||||
await runNotificationCommand(
|
||||
ctx,
|
||||
cmuxPath,
|
||||
["notify", "--title", title, "--body", message],
|
||||
(shell) => shell`${cmuxPath} notify --title ${title} --body ${message}`,
|
||||
"throw"
|
||||
)
|
||||
break
|
||||
} catch (error) {
|
||||
if (error instanceof Error) {
|
||||
logCommandFailure("cmux", error)
|
||||
} else {
|
||||
logCommandFailure("cmux", String(error))
|
||||
}
|
||||
}
|
||||
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)
|
||||
const osascriptPath = await getOsascriptPath()
|
||||
if (!osascriptPath) return
|
||||
await runNotificationCommand(
|
||||
ctx,
|
||||
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 escapedMessage = escapeAppleScriptText(message)
|
||||
await runQuietNothrow(ctx.$`${osascriptPath} -e ${"display notification \"" + escapedMessage + "\" with title \"" + escapedTitle + "\""}`)
|
||||
break
|
||||
const toastScript = buildWindowsToastScript(title, message)
|
||||
await runNotificationCommand(
|
||||
ctx,
|
||||
powershellPath,
|
||||
["-Command", toastScript],
|
||||
(shell) => shell`${powershellPath} -Command ${toastScript}`
|
||||
)
|
||||
break
|
||||
}
|
||||
}
|
||||
case "linux": {
|
||||
const notifySendPath = await getNotifySendPath()
|
||||
if (!notifySendPath) return
|
||||
|
||||
await runQuietNothrow(ctx.$`${notifySendPath} ${title} ${message} 2>/dev/null`)
|
||||
break
|
||||
}
|
||||
case "win32": {
|
||||
const powershellPath = await getPowershellPath()
|
||||
if (!powershellPath) return
|
||||
|
||||
const toastScript = buildWindowsToastScript(title, message)
|
||||
await runQuietNothrow(ctx.$`${powershellPath} -Command ${toastScript}`)
|
||||
break
|
||||
} catch (error) {
|
||||
if (error instanceof Error) {
|
||||
logOperationFailure("send", error)
|
||||
} else {
|
||||
logOperationFailure("send", String(error))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -129,33 +230,60 @@ export async function playSessionNotificationSound(
|
||||
platform: Platform,
|
||||
soundPath: string
|
||||
): Promise<void> {
|
||||
if (!canRunNotificationCommand(ctx)) return
|
||||
|
||||
switch (platform) {
|
||||
case "darwin": {
|
||||
const afplayPath = await getAfplayPath()
|
||||
if (!afplayPath) return
|
||||
await runQuietNothrow(ctx.$`${afplayPath} ${soundPath}`)
|
||||
break
|
||||
}
|
||||
case "linux": {
|
||||
const paplayPath = await getPaplayPath()
|
||||
if (paplayPath) {
|
||||
await runQuietNothrow(ctx.$`${paplayPath} ${soundPath} 2>/dev/null`)
|
||||
} else {
|
||||
const aplayPath = await getAplayPath()
|
||||
if (aplayPath) {
|
||||
await runQuietNothrow(ctx.$`${aplayPath} ${soundPath} 2>/dev/null`)
|
||||
}
|
||||
try {
|
||||
switch (platform) {
|
||||
case "darwin": {
|
||||
const afplayPath = await getAfplayPath()
|
||||
if (!afplayPath) return
|
||||
await runNotificationCommand(
|
||||
ctx,
|
||||
afplayPath,
|
||||
[soundPath],
|
||||
(shell) => shell`${afplayPath} ${soundPath}`
|
||||
)
|
||||
break
|
||||
}
|
||||
case "linux": {
|
||||
const paplayPath = await getPaplayPath()
|
||||
if (paplayPath) {
|
||||
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": {
|
||||
const powershellPath = await getPowershellPath()
|
||||
if (!powershellPath) return
|
||||
const escaped = escapePowerShellSingleQuotedText(soundPath)
|
||||
await runQuietNothrow(ctx.$`${powershellPath} -Command ${"(New-Object Media.SoundPlayer '" + escaped + "').PlaySync()"}`)
|
||||
break
|
||||
} catch (error) {
|
||||
if (error instanceof Error) {
|
||||
logOperationFailure("sound", error)
|
||||
} else {
|
||||
logOperationFailure("sound", String(error))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user