Skip to content

fix: non-macOS notification focus detection #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
terisuke merged 1 commit into from
Apr 5, 2026
Merged

fix: non-macOS notification focus detection #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
41 changes: 30 additions & 11 deletions packages/opencode/src/notification/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,37 @@ export function xml(title: string, message: string) {
}

export namespace Notification {
export async function terminalIsFocused(): Promise<boolean> {
if (platform() !== "darwin") return false
export async function terminalIsFocused(
overridePlatform?: NodeJS.Platform,
): Promise<boolean> {
const os = overridePlatform ?? platform()

const result = await Process.text(
[
"osascript",
"-e",
'tell application "System Events" to get name of first application process whose frontmost is true',
],
{ nothrow: true },
)
return terminal(result.text.trim())
if (os === "darwin") {
const result = await Process.text(
[
"osascript",
"-e",
'tell application "System Events" to get name of first application process whose frontmost is true',
],
{ nothrow: true },
)
return terminal(result.text.trim())
}

if (os === "linux") {
const result = await Process.text(
["xdotool", "getactivewindow", "getwindowpid"],
{ nothrow: true },
)
if (result.code !== 0) return true
const pid = parseInt(result.text.trim(), 10)
if (isNaN(pid)) return true
return pid === process.pid || pid === process.ppid
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

Linux focus detection compares the active window PID from xdotool getactivewindow getwindowpid to process.pid/process.ppid. That PID is the GUI window owner (e.g. gnome-terminal, alacritty, code), not the Node/Bun process running inside the terminal, so this will almost always return false even when the terminal is focused (and notifications will keep firing).

Consider mapping the returned PID to its executable/comm (e.g. read /proc/<pid>/comm or /proc/<pid>/exe, or ps -p <pid> -o comm=) and running it through the existing terminal() normalization, or alternatively use xdotool to query the active window class/name and compare via terminal().

Suggested change
return pid === process.pid || pid === process.ppid
const processResult = await Process.text(
["ps", "-p", String(pid), "-o", "comm="],
{ nothrow: true },
)
if (processResult.code !== 0) return true
const activeProcess = processResult.text.trim()
if (!activeProcess) return true
return terminal(activeProcess)

Copilot uses AI. Check for mistakes.
}

if (os === "win32") return false

return true
}

export async function show(title: string, message: string): Promise<void> {
Expand Down
72 changes: 70 additions & 2 deletions packages/opencode/test/notification.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, test } from "bun:test"
import { terminal, xml } from "../src/notification"
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"
import { terminal, xml, Notification } from "../src/notification"
import { Process } from "../src/util/process"

describe("notification", () => {
test("matches known terminal apps exactly after normalization", () => {
Expand All @@ -15,3 +16,70 @@ describe("notification", () => {
)
})
})

describe("terminalIsFocused", () => {
let origText: typeof Process.text

beforeEach(() => {
origText = Process.text
})

afterEach(() => {
;(Process as Record<string, unknown>).text = origText
})

test("returns false on win32 (allow notifications since show() works)", async () => {
const result = await Notification.terminalIsFocused("win32")
expect(result).toBe(false)
})

test("returns true on unsupported platforms (freebsd)", async () => {
const result = await Notification.terminalIsFocused("freebsd")
expect(result).toBe(true)
})

test("linux: returns true when xdotool fails (not installed)", async () => {
;(Process as Record<string, unknown>).text = mock(async () => ({
code: 1,
stdout: Buffer.alloc(0),
stderr: Buffer.from("command not found"),
text: "",
}))
const result = await Notification.terminalIsFocused("linux")
expect(result).toBe(true)
})

test("linux: returns true when xdotool returns matching PID", async () => {
;(Process as Record<string, unknown>).text = mock(async () => ({
code: 0,
stdout: Buffer.from(String(process.pid) + "\n"),
stderr: Buffer.alloc(0),
text: String(process.pid) + "\n",
}))
const result = await Notification.terminalIsFocused("linux")
expect(result).toBe(true)
})

test("linux: returns false when xdotool returns non-matching PID", async () => {
const fakePid = 99999
;(Process as Record<string, unknown>).text = mock(async () => ({
code: 0,
stdout: Buffer.from(String(fakePid) + "\n"),
stderr: Buffer.alloc(0),
text: String(fakePid) + "\n",
}))
Comment on lines +52 to +70
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

The Linux tests assume xdotool ... getwindowpid returns process.pid, but in practice that command returns the PID of the active window owner (terminal emulator / editor), not the PID of the CLI process running inside the terminal. This makes the test pass while masking the likely real-world failure mode.

After updating Linux focus detection (e.g., resolve PID -> executable name and call terminal()), adjust the tests to stub realistic xdotool output and assert against that behavior.

Suggested change
test("linux: returns true when xdotool returns matching PID", async () => {
;(Process as Record<string, unknown>).text = mock(async () => ({
code: 0,
stdout: Buffer.from(String(process.pid) + "\n"),
stderr: Buffer.alloc(0),
text: String(process.pid) + "\n",
}))
const result = await Notification.terminalIsFocused("linux")
expect(result).toBe(true)
})
test("linux: returns false when xdotool returns non-matching PID", async () => {
const fakePid = 99999
;(Process as Record<string, unknown>).text = mock(async () => ({
code: 0,
stdout: Buffer.from(String(fakePid) + "\n"),
stderr: Buffer.alloc(0),
text: String(fakePid) + "\n",
}))
test("linux: returns true when xdotool returns a terminal window PID and it resolves to a known terminal app", async () => {
const activeWindowPid = 4242
;(Process as Record<string, unknown>).text = mock(async (...args: unknown[]) => {
const command = args.map(String).join(" ")
if (command.includes("xdotool") && command.includes("getwindowpid")) {
return {
code: 0,
stdout: Buffer.from(String(activeWindowPid) + "\n"),
stderr: Buffer.alloc(0),
text: String(activeWindowPid) + "\n",
}
}
return {
code: 0,
stdout: Buffer.from("Visual Studio Code\n"),
stderr: Buffer.alloc(0),
text: "Visual Studio Code\n",
}
})
const result = await Notification.terminalIsFocused("linux")
expect(result).toBe(true)
})
test("linux: returns false when xdotool returns a window-owner PID that resolves to a non-terminal app", async () => {
const activeWindowPid = 99999
;(Process as Record<string, unknown>).text = mock(async (...args: unknown[]) => {
const command = args.map(String).join(" ")
if (command.includes("xdotool") && command.includes("getwindowpid")) {
return {
code: 0,
stdout: Buffer.from(String(activeWindowPid) + "\n"),
stderr: Buffer.alloc(0),
text: String(activeWindowPid) + "\n",
}
}
return {
code: 0,
stdout: Buffer.from("Xcode\n"),
stderr: Buffer.alloc(0),
text: "Xcode\n",
}
})

Copilot uses AI. Check for mistakes.
const result = await Notification.terminalIsFocused("linux")
expect(result).toBe(false)
})

test("linux: returns true when xdotool returns non-numeric output", async () => {
;(Process as Record<string, unknown>).text = mock(async () => ({
code: 0,
stdout: Buffer.from("not-a-number\n"),
stderr: Buffer.alloc(0),
text: "not-a-number\n",
}))
const result = await Notification.terminalIsFocused("linux")
expect(result).toBe(true)
})
})
Loading