Skip to content

Add opt-in terminal notifications for Studio Code (Proof of Concept) - #4179

Draft
SumoTTo wants to merge 5 commits into
Automattic:trunkfrom
SumoTTo:add-agent-notifications
Draft

Add opt-in terminal notifications for Studio Code (Proof of Concept)#4179
SumoTTo wants to merge 5 commits into
Automattic:trunkfrom
SumoTTo:add-agent-notifications

Conversation

@SumoTTo

@SumoTTo SumoTTo commented Jul 12, 2026 •
edited by sejas
Loading

Copy link
Copy Markdown

Related issues

How AI was used in this PR

This entire PR — code, tests, and this description — was written in an AI pair-programming session (Studio Code itself, dogfooding the agent on its own CLI source). I reviewed
every file change myself: read each diff, questioned the design decisions (why two output adapters need separate hooks, why stderr over stdout, why the --json guard checks both
process.send and isTTY), and manually verified the terminal behavior across Warp/Git Bash/PowerShell/CMD myself rather than trusting the agent's claims. I did not write the
implementation by hand.

This is a Proof of Concept (see AGENTS.md's "Large & Exploratory Contributions" section) — opened as a draft since I couldn't apply the Proof of Concept label myself as
an external contributor. Please add it if a maintainer agrees this is worth exploring further.

Proposed Changes

Adds a terminal notification (OSC 9 written to stderr) when a studio code agent turn finishes or pauses on an AskUserQuestion, so users don't have to keep watching the
terminal during long-running turns.

Auto-detected: on by default for terminals confirmed to render OSC 9 as a system notification — Warp, iTerm2, WezTerm, Ghostty, Kitty — and silent everywhere else. The
/notifications slash command cycles a manual override on top of detection: auto-detect → always on → always off → back to auto-detect.

No bare BEL: earlier versions of this PR also wrote a bare BEL as a fallback. Dropped per review feedback.

Testing Instructions

  1. Run npm run cli:build && node apps/cli/dist/cli/main.mjs code
  2. Run /notifications a few times — it should cycle through "auto-detect", "always on", and "always off" messages.
  3. On a capable terminal (Warp, iTerm2, WezTerm, Ghostty, Kitty), send a message and wait for the turn to finish, or ask something that triggers AskUserQuestion — you should
    see a system notification.
  4. Set /notifications to "always off" and confirm no signal on the next turn, even on a capable terminal.
  5. Unit tests: npm test -- apps/cli/lib/tests/notify.test.ts apps/cli/ai/tests/output-adapter.test.ts apps/cli/ai/tests/ui.test.ts apps/cli/ai/tests/slash-commands.test.ts

Manual terminal verification, on Windows: Warp shows a system notification, both on auto-detect and with "always on". Windows Terminal and ConEmu show nothing even with
"always on" forced — both use the OSC 9 slot for a progress indicator, not notifications, so this is expected and correctly excluded from auto-detect. Git Bash, PowerShell, and
CMD show nothing either — none of them render OSC 9 as a notification. macOS/Linux terminals (iTerm2, WezTerm, Ghostty, Kitty) are in the auto-detect list based on source/docs
review but untested by me directly — would appreciate confirmation from someone on those platforms.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?
@webloftdesigns-kirill
Add opt-in terminal notifications utility
995219b 
Adds notifyTerminal(), a small best-effort helper that writes BEL +
OSC 9 to stderr so terminals that support it (Warp, iTerm2, etc.) can
flash a tab or show a system notification. Off by default via a new
notificationsEnabled cli.json flag so existing users see no behavior
change.

Suppressed when forked by the Studio desktop app over IPC (it has its
own native OS notification path) or when stderr isn't a TTY (headless
spawns, e.g. the remote-session daemon). Uses stderr rather than
stdout because stdout is the documented NDJSON event stream for
`studio code --json`, matching the existing split in tos-notice.ts.

Not yet wired into any call sites.
@webloftdesigns-kirill
Wire notifications into headless and interactive agent output
a19bd3e 
Calls notifyTerminal() from both AiOutputAdapter implementations when
a turn finishes or pauses to ask the user a question:

- JsonAdapter (studio code --json, headless NDJSON mode): on
  emitTurnCompleted's 'success' and 'paused' statuses.
- AiChatUI (interactive TUI): on askUser() and the agent_end success
  branch.

JsonAdapter is also used by non-interactive callers (the desktop app
via fork+IPC, the remote-session/Telegram daemon via spawn+pipes), but
notifyTerminal's own guards (process.send, stderr.isTTY) make sure
those never see raw escape codes — only a human running `studio code
--json` directly in their own terminal does.
@webloftdesigns-kirill
Add /notifications slash command to toggle the feature
3f642f0 
Lets users enable or disable terminal notifications from inside
studio code, alongside /model and /provider. Off by default, so this
is the only way to opt in.
@youknowriad

youknowriad commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Thanks a lot for this PR — and for the thorough description and cross-terminal testing notes. The implementation itself is in good shape: the stderr/TTY/IPC guards are right, the config write goes through the proper locking helpers, and it's nicely scoped as an opt-in PoC.

One ask before this moves forward: could you trim the lower-value tests? The coverage is currently optimized for volume rather than for what can actually break. Specifically:

  • setNotificationsEnabled persists true/false — these assert that a one-line wrapper calls the function it wraps.
  • areNotificationsEnabled with explicit true/false — testing === true on a boolean; the "flag absent → false" case is the only interesting one and worth keeping.
  • is registered with a handler and a discoverable description — asserts the object literal exists.
  • The autocomplete test — getActiveSlashCommands() is just a sort of the same array, so this re-tests the registration assertion; it wouldn't catch actual autocomplete breakage.
  • The enable/disable toggle tests in slash-commands.test.ts — keeping one of the two is enough to guard the message/state inversion case.

Definitely keep the double-notification regression test in output-adapter.test.ts and the guard tests in notify.test.ts (silent under desktop IPC, silent on non-TTY stderr, never throws on config-read failure) — those encode the real contract of the feature and its subtlest behavior.

Thanks again for dogfooding Studio Code on its own source!
@youknowriad

youknowriad commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

One more thought on the opt-in/opt-out question — after looking at how Claude Code handles the same problem, I'd suggest inverting the default, with a guard:

Proposal: on by default, but only in terminals known to support OSC 9 well — and drop the bell entirely.

Claude Code's default (preferredNotifChannel: "auto") works this way: it sends desktop notifications automatically, but only in terminals it positively detects as handling them well (iTerm2, Ghostty, Kitty); in every other terminal, "auto" does nothing and the user has to opt in explicitly. Notably, the audible bell is never on by default there — it's its own explicit channel.

Applied here, that would mean:

  1. Detect the terminal (e.g. TERM_PROGRAM / TERM against a known-good list — iTerm2, WezTerm, Ghostty, Kitty, Warp…) and enable notifications by default only on that list. Everywhere else, stay silent unless the user turns it on via /notifications.
  2. Drop the bare BEL (\x07 on its own). It's what makes default-on risky: on unsupported terminals it means an audible beep after every single response. Without it, the OSC 9 sequence is quiet — ignored by terminals that don't understand it — which is exactly what makes the opt-out default safe. (This does lose the Git Bash tab-flash you observed, but Git Bash wouldn't be on the known-good list anyway, and users there can still opt in.)
  3. Keep /notifications as a tri-state override on top of the detection: unset → auto (detection decides), true → always, false → never.

The upside is that the feature actually gets used — most users will never discover an off-by-default flag buried behind a slash command, and on iTerm2/Warp/Ghostty the notification is unobtrusive enough that on-by-default is the right call. The detection guard is what earns that default.

Curious what you think — happy to keep it opt-in for the PoC and treat this as the graduation criteria instead.
@youknowriad
youknowriad requested a review from sejas July 13, 2026 09:03
sejas
sejas reviewed Jul 15, 2026
View reviewed changes

@sejas sejas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@SumoTTo , thanks for adding this feature to Studio. I like the idea of receiving notifications and that it's an opt-in feature.
I tested the PR on Mac by running /notifications in Studio Code CLI and telling the agent to ask a question. It worked well. The PR is in good shape. I triggered the CI tests to confirm nothing is broken.

I see in the future we could consider playing a different sound to differentiate from other alerts, but that's just my personal preference and something we can discuss after merging the PR as a follow-up.

Image Image

@SumoTTo, would you like to keep pushing forward the PR and address Riad's feedback #4179 (comment)?

Let me know if you run into any issues, Thanks!
@webloftdesigns-kirill
Trim low-value tests per review feedback
f1fbbee 
Per @youknowriad's review on PR Automattic#4179: remove tests that only assert
an object literal exists or that a one-line wrapper calls the
function it wraps, and drop the redundant half of duplicate
true/false and enable/disable pairs. Keep the guard tests (silent
under desktop IPC, silent on non-TTY stderr, never throws on config
read failure) and the double-notification regression test, which
encode the feature's real contract.
@webloftdesigns-kirill
Auto-detect notification-capable terminals, drop bare BEL
58bb4d6 
Per @youknowriad's suggestion on PR Automattic#4179: default to on for terminals
known to render OSC 9 well (Warp, iTerm2, WezTerm, Ghostty, Kitty),
rather than requiring users to discover the /notifications command.
The command cycles a three-way override on top of detection: unset
(auto-detect) -> true (always on) -> false (always off) -> back to
unset, so users can always return to auto-detection without editing
the config file by hand.

Also drops the bare BEL write. It was the riskier half of the escape
sequence for an on-by-default feature, since unsupported terminals
may render it as an audible beep rather than ignoring it silently
like OSC 9. The BEL terminator inside the OSC 9 sequence itself is
unchanged -- it's part of the protocol, not a standalone beep.
@SumoTTo

SumoTTo commented Jul 18, 2026

Copy link
Copy Markdown
Author

Thanks a lot for this PR — and for the thorough description and cross-terminal testing notes. The implementation itself is in good shape: the stderr/TTY/IPC guards are right, the config write goes through the proper locking helpers, and it's nicely scoped as an opt-in PoC...

@youknowriad Thank you for the review and for such a detailed comment. I have removed/kept the tests as you requested.

One more thought on the opt-in/opt-out question — after looking at how Claude Code handles the same problem, I'd suggest inverting the default, with a guard:

Took this feedback and reworked the detection model:

  • Notifications now auto-detect based on the terminal, on by default for terminals confirmed to render OSC 9 as a system notification (Warp, iTerm2, WezTerm, Ghostty, Kitty),
    silent everywhere else.
  • /notifications now cycles a manual override on top of that: auto-detect → always on → always off → back to auto-detect. So anyone who disagrees with the detection (or is on
    a terminal I haven't verified) can force it either way.
  • Dropped the bare BEL fallback entirely.
@SumoTTo

SumoTTo commented Jul 18, 2026

Copy link
Copy Markdown
Author

@sejas Thank you for the review, and it's great that this works on Mac! I've made the changes requested by @youknowriad, and I will continue working on this PR. Regarding the audio signal setting, as far as I understand, unfortunately we cannot do that in this implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OSS Citizen

4 participants

@SumoTTo @youknowriad @sejas @webloftdesigns-kirill