fix(notifications): prefer terminal-notifier over osascript on macOS (#2633)
osascript display notification is silently swallowed by macOS when the calling terminal app (Ghostty, iTerm2, etc.) lacks notification permissions in System Settings. The command exits 0 with no error, making the failure invisible. terminal-notifier registers as its own Notification Center app, so macOS prompts the user for permission on first use — the expected UX. Changes: - Add findExecutable() helper to locate terminal-notifier on PATH - buildDesktopNotificationCommand() prefers terminal-notifier when available, falls back to osascript (preserving existing behavior) - Update tests to handle both terminal-notifier and osascript paths - Add macOS delivery note to docs/configuration.md notifications section - Add troubleshooting entry for notifications not appearing on macOS Fixes #2632 Co-authored-by: Yang Yang(NYC) <Yang.Yang2@bcg.com>
This commit is contained in:
parent
bbd9468c78
commit
665e2aa1cb
4 changed files with 89 additions and 6 deletions
|
|
@ -494,6 +494,14 @@ notifications:
|
|||
on_attention: true # notify when manual attention needed
|
||||
```
|
||||
|
||||
**macOS delivery:** GSD uses [`terminal-notifier`](https://github.com/julienXX/terminal-notifier) when available, falling back to `osascript`. We recommend installing `terminal-notifier` for reliable notification delivery:
|
||||
|
||||
```bash
|
||||
brew install terminal-notifier
|
||||
```
|
||||
|
||||
Why: `osascript display notification` is attributed to your terminal app (Ghostty, iTerm2, etc.), which may not have notification permissions in System Settings → Notifications. `terminal-notifier` registers as its own app and prompts for permission on first use. See [Troubleshooting: Notifications not appearing on macOS](troubleshooting.md#notifications-not-appearing-on-macos) if notifications aren't working.
|
||||
|
||||
### `remote_questions`
|
||||
|
||||
Route interactive questions to Slack or Discord for headless auto mode:
|
||||
|
|
|
|||
|
|
@ -381,3 +381,33 @@ This shows which servers are active and, if none are found, diagnoses why — in
|
|||
| Go | `go install golang.org/x/tools/gopls@latest` |
|
||||
|
||||
After installing, run `lsp reload` to restart detection without restarting GSD.
|
||||
|
||||
## Notifications
|
||||
|
||||
### Notifications not appearing on macOS
|
||||
|
||||
**Symptoms:** `notifications.enabled: true` in preferences, but no desktop notifications appear during auto-mode (no milestone complete alerts, no budget warnings, no error notifications). No error messages logged.
|
||||
|
||||
**Cause:** GSD uses `osascript display notification` as a fallback on macOS. This command is attributed to your terminal app (Ghostty, iTerm2, Alacritty, Kitty, Warp, etc.). If that app doesn't have notification permissions in System Settings → Notifications, macOS silently drops the notification — `osascript` exits 0 with no error.
|
||||
|
||||
Most terminal apps don't appear in the Notifications settings panel until they've successfully delivered at least one notification, creating a chicken-and-egg problem.
|
||||
|
||||
**Fix (recommended):** Install `terminal-notifier`, which registers as its own Notification Center app:
|
||||
|
||||
```bash
|
||||
brew install terminal-notifier
|
||||
```
|
||||
|
||||
GSD automatically prefers `terminal-notifier` when available. On first use, macOS will prompt you to allow notifications — this is the expected behavior.
|
||||
|
||||
**Fix (alternative):** Go to **System Settings → Notifications** and enable notifications for your terminal app. If your terminal doesn't appear in the list, try sending a test notification from Terminal.app first to register "Script Editor":
|
||||
|
||||
```bash
|
||||
osascript -e 'display notification "test" with title "GSD"'
|
||||
```
|
||||
|
||||
**Verify:** After applying either fix, test with:
|
||||
|
||||
```bash
|
||||
terminal-notifier -title "GSD" -message "working!" -sound Glass
|
||||
```
|
||||
|
|
|
|||
|
|
@ -74,6 +74,17 @@ export function buildDesktopNotificationCommand(
|
|||
const normalizedMessage = normalizeNotificationText(message);
|
||||
|
||||
if (platform === "darwin") {
|
||||
// Prefer terminal-notifier: registers as its own Notification Center app,
|
||||
// so it gets a proper permission entry in System Settings → Notifications.
|
||||
// osascript notifications are silently swallowed when the calling terminal
|
||||
// (Ghostty, iTerm2, etc.) lacks notification permissions — exits 0, no error.
|
||||
// See: https://github.com/gsd-build/gsd-2/issues/2632
|
||||
const tnPath = findExecutable("terminal-notifier");
|
||||
if (tnPath) {
|
||||
const sound = level === "error" ? "Basso" : "Glass";
|
||||
return { file: tnPath, args: ["-title", normalizedTitle, "-message", normalizedMessage, "-sound", sound] };
|
||||
}
|
||||
// Fallback: osascript (works if terminal app has notification permissions)
|
||||
const sound = level === "error" ? 'sound name "Basso"' : 'sound name "Glass"';
|
||||
const script = `display notification "${escapeAppleScript(normalizedMessage)}" with title "${escapeAppleScript(normalizedTitle)}" ${sound}`;
|
||||
return { file: "osascript", args: ["-e", script] };
|
||||
|
|
@ -94,3 +105,15 @@ function normalizeNotificationText(s: string): string {
|
|||
function escapeAppleScript(s: string): string {
|
||||
return s.replace(/\\/g, "\\\\").replace(/"/g, '\\"');
|
||||
}
|
||||
|
||||
/**
|
||||
* Locate an executable on PATH. Returns absolute path or null.
|
||||
* Non-fatal — returns null on any error.
|
||||
*/
|
||||
function findExecutable(name: string): string | null {
|
||||
try {
|
||||
return execFileSync("which", [name], { timeout: 2000, stdio: ["ignore", "pipe", "ignore"] }).toString().trim() || null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,7 +31,10 @@ test("shouldSendDesktopNotification disables all categories when notifications a
|
|||
assert.equal(shouldSendDesktopNotification("milestone", prefs), false);
|
||||
});
|
||||
|
||||
test("buildDesktopNotificationCommand uses argument arrays for macOS notifications", () => {
|
||||
test("buildDesktopNotificationCommand falls back to osascript on macOS when terminal-notifier is absent", () => {
|
||||
// When terminal-notifier is not on PATH, falls back to osascript.
|
||||
// This test runs in CI where terminal-notifier is typically not installed.
|
||||
// If terminal-notifier IS installed, we verify it returns that instead.
|
||||
const command = buildDesktopNotificationCommand(
|
||||
"darwin",
|
||||
`Bob's "Milestone"`,
|
||||
|
|
@ -40,11 +43,30 @@ test("buildDesktopNotificationCommand uses argument arrays for macOS notificatio
|
|||
);
|
||||
|
||||
assert.ok(command);
|
||||
assert.equal(command.file, "osascript");
|
||||
assert.deepEqual(command.args.slice(0, 1), ["-e"]);
|
||||
assert.match(command.args[1], /Bob's \\"Milestone\\"/);
|
||||
assert.match(command.args[1], /Budget! Path: C:\\\\temp/);
|
||||
assert.doesNotMatch(command.args[1], /\n/);
|
||||
if (command.file.includes("terminal-notifier")) {
|
||||
// terminal-notifier path — verify args structure
|
||||
assert.ok(command.args.includes("-title"));
|
||||
assert.ok(command.args.includes("-message"));
|
||||
assert.ok(command.args.includes("-sound"));
|
||||
assert.ok(command.args.includes("Basso")); // error level
|
||||
} else {
|
||||
// osascript fallback path
|
||||
assert.equal(command.file, "osascript");
|
||||
assert.deepEqual(command.args.slice(0, 1), ["-e"]);
|
||||
assert.match(command.args[1], /Bob's \\"Milestone\\"/);
|
||||
assert.match(command.args[1], /Budget! Path: C:\\\\temp/);
|
||||
assert.doesNotMatch(command.args[1], /\n/);
|
||||
}
|
||||
});
|
||||
|
||||
test("buildDesktopNotificationCommand uses Glass sound for non-error on macOS", () => {
|
||||
const command = buildDesktopNotificationCommand("darwin", "Title", "Message", "info");
|
||||
assert.ok(command);
|
||||
if (command.file.includes("terminal-notifier")) {
|
||||
assert.ok(command.args.includes("Glass"));
|
||||
} else {
|
||||
assert.match(command.args[1], /sound name "Glass"/);
|
||||
}
|
||||
});
|
||||
|
||||
test("buildDesktopNotificationCommand preserves literal shell characters on linux", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue