fix(tui): detect VTE-based terminals via VTE_VERSION for OSC 9 notifications#3686
fix(tui): detect VTE-based terminals via VTE_VERSION for OSC 9 notifications#3686PGupta-Git wants to merge 1 commit into
Conversation
…cations Ptyxis, GNOME Terminal, Tilix, and other VTE-based terminals set VTE_VERSION in the process environment but never set TERM_PROGRAM. detectTerminalId() checked TERM_PROGRAM before falling through to the generic COLORTERM=truecolor branch, which resolves to the 'trueColor' profile whose notifyProtocol is NotifyProtocol.Bell (\x07). VTE does not surface a bare BEL as a desktop notification, so completion.notify and ask.notify toasts were silently dropped on every VTE-based terminal. Add a VTE_VERSION check before the COLORTERM fallback. When present it resolves to a new 'vte' TerminalId whose notifyProtocol is NotifyProtocol.Osc9. VTE >= 6800 implements OSC 9 desktop notifications; the version shipped with Fedora 40+ (8400) qualifies. No Kitty graphics or DECCARA is claimed for VTE. Fixes can1357#3685. Related: can1357#1799 fixed the same detection gap for the eagerEraseScrollbackRisk path but did not update notification detection.
roboomp
left a comment
roboomp
left a comment
There was a problem hiding this comment.
P1: focused fix for the Ptyxis/VTE notification path, with changelog and contract tests, but the detection needs a version-bound decision because the code applies OSC 9 to every VTE_VERSION while documenting support only for VTE >= 6800.
One should-fix inline finding on that gate. I also ran bun test packages/tui/test/terminal-capabilities.test.ts packages/tui/test/notifications.test.ts; 50 passed and 3 existing ProcessTerminal cases failed on nativeSetHangulCompatJamoWidthOverride being undefined in this worktree, not on the new VTE assertions.
Thanks for the tight reproduction and fix.
| // VTE-based terminals (Ptyxis, GNOME Terminal, Tilix, …) set VTE_VERSION | ||
| // but not TERM_PROGRAM. Check this before the generic COLORTERM=truecolor | ||
| // fallback so they get OSC 9 notifications instead of a silent BEL. | ||
| if (VTE_VERSION) return "vte"; |
There was a problem hiding this comment.
should-fix: this treats any non-empty VTE_VERSION as OSC 9-capable, but the new profile documents support only for VTE >= 6800. A terminal exporting VTE_VERSION=6400 now resolves to "vte" and sends OSC 9 instead of keeping the previous trueColor/BEL fallback. Please gate this branch on the numeric VTE version or document/test that older VTE releases are intentionally moved to OSC 9 too.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf93bb8376
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // VTE_VERSION. They do not set TERM_PROGRAM. VTE ≥ 6800 supports OSC 9 | ||
| // desktop notifications; use Osc9 so completion/ask toasts reach the | ||
| // desktop. No Kitty graphics or DECCARA on VTE. | ||
| vte: new TerminalInfo("vte", null, true, true, NotifyProtocol.Osc9), |
There was a problem hiding this comment.
When running in Ptyxis/GNOME Terminal/Tilix with VTE_VERSION set, this profile makes TerminalInfo.formatNotification() emit ESC]9;<message>ST. VTE's OSC 9 handler is for ConEmu-style subcommands such as 9;4 progress, not arbitrary desktop notification text, so completion.notify and ask.notify still get ignored in the exact VTE environments this change is meant to fix; the VTE path needs to emit a VTE-supported notification sequence instead of NotifyProtocol.Osc9.
Useful? React with 👍 / 👎.
|
Closing in favour of #3687 / the analysis in its comments. OSC 9 on VTE is ConEmu's progress extension, not desktop notifications — the fix was wrong. VTE has no escape sequence that fires an arbitrary desktop notification today (OSC 777 also doesn't work for the reason explained by @roboomp). Leaving #3685 open as a tracking issue. |
2 participants
What
Add
VTE_VERSIONdetection todetectTerminalId()and a new"vte"terminal profile withNotifyProtocol.Osc9.Why
Fixes #3685.
Ptyxis, GNOME Terminal, Tilix, and other VTE-based terminals set
VTE_VERSIONbut never setTERM_PROGRAM.detectTerminalId()skipped straight to theCOLORTERM=truecolorfallback →"trueColor"profile →notifyProtocol = NotifyProtocol.Bell(\x07). VTE does not surface a bare BEL as a desktop notification, socompletion.notifyandask.notifytoasts were silently dropped on every VTE-based terminal on Linux.Same root cause as #1799 (fixed for
eagerEraseScrollbackRisk), now fixed for the notification path.Changes
packages/tui/src/terminal-capabilities.ts"vte"to theTerminalIdunionvteentry toKNOWN_TERMINALSwithNotifyProtocol.Osc9(no Kitty graphics, no DECCARA — VTE does not support them)VTE_VERSIONto the destructured env indetectTerminalId()and inserted the check before theCOLORTERMfallbackpackages/tui/test/terminal-capabilities.test.tsdetectTerminalIdwithVTE_VERSIONresolves to"vte"VTE_VERSIONbeatsCOLORTERM=truecolor(the exact Ptyxis env)VTE_VERSION, bareCOLORTERM=truecolorstill resolves to"trueColor"(no regression)vteprofile hasnotifyProtocol === NotifyProtocol.Osc9packages/tui/test/notifications.test.tssendNotificationwithOsc9protocol emits\x1b]9;<msg>\x1b\\(not a silent BEL)packages/tui/CHANGELOG.md[Unreleased]Testing
bun checkpasses