Merge pull request #2166 from 1noilimrev/fix/macos-notification-click-target
fix(hooks): use terminal-notifier for macOS notification click-to-focus
This commit is contained in:
@@ -7,6 +7,7 @@ import {
|
||||
getAfplayPath,
|
||||
getPaplayPath,
|
||||
getAplayPath,
|
||||
getTerminalNotifierPath,
|
||||
} from "./session-notification-utils"
|
||||
import { buildWindowsToastScript, escapeAppleScriptText, escapePowerShellSingleQuotedText } from "./session-notification-formatting"
|
||||
|
||||
@@ -39,6 +40,19 @@ export async function sendSessionNotification(
|
||||
): Promise<void> {
|
||||
switch (platform) {
|
||||
case "darwin": {
|
||||
// Try terminal-notifier first — deterministic click-to-focus
|
||||
const terminalNotifierPath = await getTerminalNotifierPath()
|
||||
if (terminalNotifierPath) {
|
||||
const bundleId = process.env.__CFBundleIdentifier
|
||||
const args = [terminalNotifierPath, "-title", title, "-message", message]
|
||||
if (bundleId) {
|
||||
args.push("-activate", bundleId)
|
||||
}
|
||||
await ctx.$`${args}`.catch(() => {})
|
||||
break
|
||||
}
|
||||
|
||||
// Fallback: osascript (click may open Finder instead of terminal)
|
||||
const osascriptPath = await getOsascriptPath()
|
||||
if (!osascriptPath) return
|
||||
|
||||
|
||||
@@ -32,11 +32,13 @@ export const getPowershellPath = createCommandFinder("powershell")
|
||||
export const getAfplayPath = createCommandFinder("afplay")
|
||||
export const getPaplayPath = createCommandFinder("paplay")
|
||||
export const getAplayPath = createCommandFinder("aplay")
|
||||
export const getTerminalNotifierPath = createCommandFinder("terminal-notifier")
|
||||
|
||||
export function startBackgroundCheck(platform: Platform): void {
|
||||
if (platform === "darwin") {
|
||||
getOsascriptPath().catch(() => {})
|
||||
getAfplayPath().catch(() => {})
|
||||
getTerminalNotifierPath().catch(() => {})
|
||||
} else if (platform === "linux") {
|
||||
getNotifySendPath().catch(() => {})
|
||||
getPaplayPath().catch(() => {})
|
||||
|
||||
@@ -365,4 +365,87 @@ describe("session-notification", () => {
|
||||
// then - only one notification should be sent
|
||||
expect(notificationCalls).toHaveLength(1)
|
||||
})
|
||||
|
||||
function createSenderMockCtx() {
|
||||
const notifyCalls: string[] = []
|
||||
const mockCtx = {
|
||||
$: async (cmd: TemplateStringsArray | string, ...values: any[]) => {
|
||||
const cmdStr = typeof cmd === "string"
|
||||
? cmd
|
||||
: cmd.reduce((acc, part, i) => acc + part + (values[i] ?? ""), "")
|
||||
notifyCalls.push(cmdStr)
|
||||
return { stdout: "", stderr: "", exitCode: 0 }
|
||||
},
|
||||
} as any
|
||||
return { mockCtx, notifyCalls }
|
||||
}
|
||||
|
||||
test("should use terminal-notifier with -activate when available on darwin", async () => {
|
||||
// given - terminal-notifier is available and __CFBundleIdentifier is set
|
||||
spyOn(sender, "sendSessionNotification").mockRestore()
|
||||
const { mockCtx, notifyCalls } = createSenderMockCtx()
|
||||
spyOn(utils, "getTerminalNotifierPath").mockResolvedValue("/usr/local/bin/terminal-notifier")
|
||||
const originalEnv = process.env.__CFBundleIdentifier
|
||||
process.env.__CFBundleIdentifier = "com.mitchellh.ghostty"
|
||||
|
||||
try {
|
||||
// when - sendSessionNotification is called directly on darwin
|
||||
await sender.sendSessionNotification(mockCtx, "darwin", "Test Title", "Test Message")
|
||||
|
||||
// then - notification uses terminal-notifier with -activate flag
|
||||
expect(notifyCalls.length).toBeGreaterThanOrEqual(1)
|
||||
const tnCall = notifyCalls.find(c => c.includes("terminal-notifier"))
|
||||
expect(tnCall).toBeDefined()
|
||||
expect(tnCall).toContain("-activate")
|
||||
expect(tnCall).toContain("com.mitchellh.ghostty")
|
||||
} finally {
|
||||
if (originalEnv !== undefined) {
|
||||
process.env.__CFBundleIdentifier = originalEnv
|
||||
} else {
|
||||
delete process.env.__CFBundleIdentifier
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
test("should fall back to osascript when terminal-notifier is not available", async () => {
|
||||
// given - terminal-notifier is NOT available
|
||||
spyOn(sender, "sendSessionNotification").mockRestore()
|
||||
const { mockCtx, notifyCalls } = createSenderMockCtx()
|
||||
spyOn(utils, "getTerminalNotifierPath").mockResolvedValue(null)
|
||||
spyOn(utils, "getOsascriptPath").mockResolvedValue("/usr/bin/osascript")
|
||||
|
||||
// when - sendSessionNotification is called directly on darwin
|
||||
await sender.sendSessionNotification(mockCtx, "darwin", "Test Title", "Test Message")
|
||||
|
||||
// then - notification uses osascript (fallback)
|
||||
expect(notifyCalls.length).toBeGreaterThanOrEqual(1)
|
||||
const osascriptCall = notifyCalls.find(c => c.includes("osascript"))
|
||||
expect(osascriptCall).toBeDefined()
|
||||
const tnCall = notifyCalls.find(c => c.includes("terminal-notifier"))
|
||||
expect(tnCall).toBeUndefined()
|
||||
})
|
||||
|
||||
test("should use terminal-notifier without -activate when __CFBundleIdentifier is not set", async () => {
|
||||
// given - terminal-notifier available but no bundle ID
|
||||
spyOn(sender, "sendSessionNotification").mockRestore()
|
||||
const { mockCtx, notifyCalls } = createSenderMockCtx()
|
||||
spyOn(utils, "getTerminalNotifierPath").mockResolvedValue("/usr/local/bin/terminal-notifier")
|
||||
const originalEnv = process.env.__CFBundleIdentifier
|
||||
delete process.env.__CFBundleIdentifier
|
||||
|
||||
try {
|
||||
// when - sendSessionNotification is called directly on darwin
|
||||
await sender.sendSessionNotification(mockCtx, "darwin", "Test Title", "Test Message")
|
||||
|
||||
// then - terminal-notifier used but without -activate flag
|
||||
expect(notifyCalls.length).toBeGreaterThanOrEqual(1)
|
||||
const tnCall = notifyCalls.find(c => c.includes("terminal-notifier"))
|
||||
expect(tnCall).toBeDefined()
|
||||
expect(tnCall).not.toContain("-activate")
|
||||
} finally {
|
||||
if (originalEnv !== undefined) {
|
||||
process.env.__CFBundleIdentifier = originalEnv
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user