Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes Shift+Enter (and similar keyboard chords) in TUIs like claude-code by changing
Confidence Score: 5/5Safe to merge — minimal, well-reasoned single-value change with updated tests and thorough documentation. The diff is a single string literal change backed by solid reverse-engineering of the claude-code allowlist. The PR description justifies the choice of 'kitty' over the other four allowlist values, tests are updated, and there are no logic regressions to worry about. Plain-bash degradation is graceful; the TERM_PROGRAM_VERSION mismatch is acknowledged and is effectively harmless since kitty version checks in the wild are typically lower bounds and '2.0.0' clears all of them. No files require special attention.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/terminal/env.ts | One-line fix: TERM_PROGRAM changed from "Superset" to "kitty" with an explanatory comment; no logic changes beyond this. |
| packages/host-service/src/terminal/env.test.ts | Test expectations updated to match the new TERM_PROGRAM value; coverage is unchanged. |
Sequence Diagram
sequenceDiagram
participant User
participant xterm as xterm.js
participant TUI as Shell/TUI
User->>xterm: Shift+Enter keypress
Note over xterm: vtExtensions.kittyKeyboard emits CSI-u bytes
alt Before fix - TERM_PROGRAM=Superset
xterm->>TUI: CSI-u bytes
Note over TUI: Not in allowlist, legacy readline used
TUI->>TUI: Plain Enter detected, submits form
end
alt After fix - TERM_PROGRAM=kitty
xterm->>TUI: CSI-u bytes
Note over TUI: kitty in allowlist, CSI-u parser active
TUI->>TUI: Shift+Enter detected, inserts newline
end
Reviews (2): Last reviewed commit: "chore(host-service): trim TERM_PROGRAM c..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts (2)
7-7: Consider relocatingclipboardShortcutsto a shared location.
lib/terminal/terminal-runtime.tsnow depends on a deep path inside the UI tree (screens/main/components/.../Terminal/clipboardShortcuts). Since this module is now consumed by both the runtime library and the Terminal component, per the co-location guideline it could be promoted to a common parent (e.g.lib/terminal/alongside the runtime, or a shared util). Non-blocking; just improves dependency direction (lib should not reach into a specific screen subtree).As per coding guidelines: "If a utility is used once, nest it under the parent's directory. If used 2+ times, promote it to the highest shared parent's directory or top-level
components/."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts` at line 7, The import of shouldBubbleClipboardShortcut from the deep UI path couples terminal-runtime.ts to the screen tree; move the clipboardShortcuts module to a shared location (e.g., lib/terminal/clipboardShortcuts.ts or a shared utils folder) and update both terminal-runtime.ts and the Terminal component to import shouldBubbleClipboardShortcut from that new module; ensure the exported symbol name (shouldBubbleClipboardShortcut) remains unchanged so functions like the runtime initialization and Terminal’s event handlers continue to work without further changes.
22-27: UseelectronTrpc.window.getPlatforminstead of deprecatednavigator.platform.
navigator.platformis deprecated and returns inconsistent values across runtimes. The codebase already exposes platform info via tRPC atelectronTrpc.window.getPlatform.useQuery()(defined inapps/desktop/src/lib/trpc/routers/window.ts). This pattern is already used in other renderer files (e.g., settings layout, dashboard top bar). Update this function to use the tRPC approach for consistency with the rest of the app and better maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts` around lines 22 - 27, The code in createKeyEventHandler currently reads navigator.platform (deprecated); replace this by using the existing tRPC hook electronTrpc.window.getPlatform.useQuery() instead of directly accessing navigator, or better yet change createKeyEventHandler's signature to accept a platform string (or boolean flags isMac/isWindows) so callers can pass the value obtained from electronTrpc.window.getPlatform.useQuery(); update callers to call electronTrpc.window.getPlatform.useQuery() (as used elsewhere) and pass the platform into createKeyEventHandler so createKeyEventHandler no longer uses navigator.platform.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.test.ts (1)
77-99: Expectations match the updated bubbling rules.All four updated assertions align with the new
shouldBubbleClipboardShortcutbranches (macOSonlyMeta && KeyC, WindowsonlyCtrl && hasSelectiongate preserved,ctrlShiftOnlyalways bubbles on Win/Linux).Optional: consider adding a couple more cases to lock in the intended behavior:
linux Ctrl+C without selection→expected: false(ensuresKeyCis not matched by the Linux fallthrough and stays with the PTY for SIGINT).macOS Cmd+Shift+C→expected: false(guards against accidentally widening the mac branch to includeCmd+Shift+C).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.test.ts` around lines 77 - 99, Add two tests to cover the suggested edge cases so the test suite locks in the intended branching of shouldBubbleClipboardShortcut: add a case using makeEvent({ code: "KeyC", ctrlKey: true }) with options { isMac: false, isWindows: false, hasSelection: false } expecting false (linux Ctrl+C without selection should not bubble), and add a case using makeEvent({ code: "KeyC", metaKey: true, shiftKey: true }) with options { isMac: true, isWindows: false, hasSelection: false } expecting false (macOS Cmd+Shift+C should not bubble); place them near the existing related cases so they validate onlyCtrl vs onlyMeta and ctrlShiftOnly branches in shouldBubbleClipboardShortcut.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/desktop/src/renderer/lib/terminal/terminal-runtime.ts`:
- Line 7: The import of shouldBubbleClipboardShortcut from the deep UI path
couples terminal-runtime.ts to the screen tree; move the clipboardShortcuts
module to a shared location (e.g., lib/terminal/clipboardShortcuts.ts or a
shared utils folder) and update both terminal-runtime.ts and the Terminal
component to import shouldBubbleClipboardShortcut from that new module; ensure
the exported symbol name (shouldBubbleClipboardShortcut) remains unchanged so
functions like the runtime initialization and Terminal’s event handlers continue
to work without further changes.
- Around line 22-27: The code in createKeyEventHandler currently reads
navigator.platform (deprecated); replace this by using the existing tRPC hook
electronTrpc.window.getPlatform.useQuery() instead of directly accessing
navigator, or better yet change createKeyEventHandler's signature to accept a
platform string (or boolean flags isMac/isWindows) so callers can pass the value
obtained from electronTrpc.window.getPlatform.useQuery(); update callers to call
electronTrpc.window.getPlatform.useQuery() (as used elsewhere) and pass the
platform into createKeyEventHandler so createKeyEventHandler no longer uses
navigator.platform.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.test.ts`:
- Around line 77-99: Add two tests to cover the suggested edge cases so the test
suite locks in the intended branching of shouldBubbleClipboardShortcut: add a
case using makeEvent({ code: "KeyC", ctrlKey: true }) with options { isMac:
false, isWindows: false, hasSelection: false } expecting false (linux Ctrl+C
without selection should not bubble), and add a case using makeEvent({ code:
"KeyC", metaKey: true, shiftKey: true }) with options { isMac: true, isWindows:
false, hasSelection: false } expecting false (macOS Cmd+Shift+C should not
bubble); place them near the existing related cases so they validate onlyCtrl vs
onlyMeta and ctrlShiftOnly branches in shouldBubbleClipboardShortcut.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3a6804f5-2039-4f12-8b61-d5ee41dd99f0
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
apps/desktop/src/renderer/lib/terminal/terminal-runtime.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/clipboardShortcuts.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Kitenite
changed the title
Kitenite
force-pushed
the
did-we-regress-something-in-terminal-recently-for-tui-cmdc-in-tuis-in-terminal-v2-now-outputs-a-char
branch
from
f58cfcb to
0bc1d0a
Compare
v2's terminal runtime only filtered app hotkeys. With kitty keyboard protocol enabled (needed for Shift+Enter disambiguation in claude-code, modifier reporting in neovim/helix), every Mac Cmd chord xterm saw got CSI-u encoded and leaked into TUIs as a literal char — and line-edit niceties like Cmd+Left/Right/Backspace and Option+Left/Right that v1 handles never worked at all. Mirror Ghostty's approach (src/input/key_encode.zig:534-545: "on macOS, command+keys do not encode text"): bubble every Mac Cmd chord out to the host before xterm's kitty encoder runs, then port v1's line-edit chord translators so shell navigation works the same in both renderers. Changes: - Broaden shouldBubbleClipboardShortcut's Mac branch to bubble all Cmd chords (not just Cmd+C/V with selection gating). v1 benefits too. - Port v1's line-edit translators into v2's custom key handler: Cmd+Left/Right/Backspace, Option+Left/Right, Windows Ctrl+Left/Right. Duplicates v1 for now; a follow-up can share the handler properly. - Wire shouldSelectAllShortcut into v2 so Cmd+A selects terminal buffer. - Use xterm.input(data, true) to inject translated sequences into the PTY (fires onData, forwarded by terminal-ws-transport). - Restructure tests around the new Mac rule.
Kitenite
changed the title
When claude-code or codex pushes kitty progressive-enhancement flags (CSI > N u), the running program expects modified keys as CSI-u. xterm.js v6.1-beta tracks this internally but doesn't expose the active flags, so we mirror them via our own CSI handlers registered alongside xterm's built-ins (registering with return-false passes through to xterm too). With the disambiguate bit active, Shift+Enter now emits the canonical \x1b[13;2u that both claude-code and codex recognise — matching Ghostty / kitty / wezterm, which all gate CSI-u on program-initiated kitty mode (ghostty/src/input/key_encode.zig:88, kitty/key_encoding.c:153). Without kitty flags, Shift+Enter falls through to xterm.js's legacy encoding (plain \r), so bash / zsh still behave normally. Replaces the previous alt-screen heuristic, which incorrectly missed Ink-based TUIs like claude-code that render inline rather than in the alternate buffer.
Add three toggleable diagnostic taps, all gated on localStorage flag
`__kbdDebug=1`:
- Every keydown that reaches the custom key handler: key / code / mods /
current kitty flags.
- Every kitty CSI push/set/pop with resulting flag state.
- Every onData byte written to the PTY, shown as hex (non-printable
escaped, printable verbatim) plus length and kitty flags.
Second flag `__kbdDebugSkipOverride=1` temporarily disables our
Shift+Enter CSI-u override so we can observe xterm.js's raw encoding.
To use: in DevTools console, `localStorage.setItem('__kbdDebug', '1')`
(optionally also `__kbdDebugSkipOverride`), reload the terminal pane,
reproduce the bug, copy `[kbd:*]` logs.
Diagnostic trace showed xterm.js emits event-type-suffixed kitty sequences when the running program activates the report-events flag (0x02) — e.g. Escape release came through as \x1b[27;1:3u. Our override was hardcoded to \x1b[13;2u which is the right form only when disambiguate (0x01) is the sole flag; claude-code (which requests flags = 0x07) was rejecting the suffix-less form and submitting instead of newline. Inspect the flags at inject time: emit \x1b[13;2:1u (explicit press event type) when 0x02 is active, \x1b[13;2u otherwise. Also make kbdLog stringify its payload so DevTools shows the bytes inline instead of collapsing to "Object".
Diagnostic trace of v2 terminal Shift+Enter proved xterm.js v6 with
kittyKeyboard is emitting the correct kitty-protocol bytes (\x1b[13;2u
press + \x1b[13;2:3u release, same as Ghostty). Yet Shift+Enter still
submits in claude-code — strings-dumped the claude-code binary and
found the actual gate:
Ix_ = {
ghostty: "Ghostty", kitty: "Kitty", "iTerm.app": "iTerm2",
WezTerm: "WezTerm", WarpTerminal: "Warp"
}
Its CSI-u parser keys off TERM_PROGRAM being in that allowlist. With
our previous "Superset" value it ignored the bytes entirely and fell
through to Node readline's legacy keypress handling, where Shift+Enter
looks like plain Enter and submits.
Claim "kitty" — the protocol origin. Fewer downstream version-gated
branches than "ghostty" (which claude-code version-checks against 1.2.0)
and safer than "iTerm.app" (color-depth gated on version <3.x).
TERM_PROGRAM_VERSION stays as our host-service version; programs that
care about real kitty version will hit our version string and probably
fall back to conservative behaviour.
…ide code Root cause for claude-code Shift+Enter landed in host-service env.ts (TERM_PROGRAM=kitty). The renderer-side machinery we added while diagnosing is no longer needed: - kbdLog / kbdHex / kbdDebugSkipOverride console tap - createKittyFlagTracker (watched CSI > u pushes to gate the override) - KITTY_FLAG_DISAMBIGUATE / KITTY_FLAG_REPORT_EVENTS constants - shiftEnterCsiU helper - terminal.onData logging tap v2 terminal-runtime.ts is back to a simple createKeyEventHandler calling resolveHotkeyFromEvent → line-edit translators → select-all → clipboard bubble → xterm default. Shift+Enter now goes through xterm's native kitty encoder and claude-code parses it correctly because TERM_PROGRAM is in its allowlist. Net -142 LOC from renderer.
Kitenite
changed the title
Hypothesis: with TERM_PROGRAM=kitty claiming kitty in the claude-code / codex allowlist, kitty-aware TUIs parse CSI-u Cmd+chords correctly and don't show literal char garbage. The broader Ghostty-style "bubble every Mac Cmd chord" may have been overkill — needed only for TUIs that don't speak CSI-u. Revert the Mac branch of shouldBubbleClipboardShortcut to the original narrow rule (Cmd+V + Cmd+C-with-selection) to test. If kitty-aware TUIs stay clean AND non-kitty-aware ones (vim base, less, htop, tmux-without- kitty) are tolerable, we can ship simpler. Line-edit translators and Cmd+A select-all stay — those aren't kitty-specific, they're Mac-convention handling the shell expects regardless of kitty.
…RAM=kitty
Aggressive minimum — strip terminal-runtime.ts and clipboardShortcuts.ts
back to main. The only net change from main becomes:
env.TERM_PROGRAM = "kitty" (was "Superset")
If claude-code's CSI-u parsing handles Cmd+C/V/Left/Right correctly with
TERM_PROGRAM now in the allowlist, and shell fallback is acceptable for
the rest, this is the true minimum fix.
Things that may regress and be worth checking:
- Cmd+Left / Cmd+Right / Cmd+Backspace in shell (no translator → xterm
sends CSI-u → bash readline ignores → no cursor navigation)
- Option+Left / Option+Right (same, word navigation won't work in shell)
- Cmd+A in shell (no select-all handler → CSI-u to PTY)
- Cmd+C in vim / less / htop (no broad bubble → CSI-u reaches PTY; if
those TUIs don't speak kitty they'll show garbage)
If any of those bite, we restore the specific piece(s).
Kitenite
changed the title
This was referenced Apr 23, 2026
Kitenite
deleted the
did-we-regress-something-in-terminal-recently-for-tui-cmdc-in-tuis-in-terminal-v2-now-outputs-a-char
branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The fix
One env var.
packages/host-service/src/terminal/env.ts:Why
Original bug: Shift+Enter in claude-code submits instead of inserting a newline. xterm.js v6 with
vtExtensions.kittyKeyboard: truealready emits the correct kitty CSI-u bytes (\x1b[13;2upress +\x1b[13;2:3urelease — verified against Ghostty's own test atghostty/src/input/key_encode.zig:1370). The gap was in claude-code:strings-dumping the binary revealed it only activates its CSI-u parser whenTERM_PROGRAMis in this allowlist:With our previous
TERM_PROGRAM=Superset, claude-code fell through to Node readline's legacy keypress handling, which can't distinguish Shift+Enter from Enter — so Shift+Enter submitted. Claimingkittyhits the allowlist and claude-code parses our bytes correctly.Same mechanism fixes other reported annoyances (Cmd+C leaking a literal 'c' in TUIs, missing Cmd+Left/Right navigation, missing Cmd+A select-all) — once claude-code/codex parse CSI-u, they handle those chords themselves, and kitty-aware shells (fish 4+, zsh with kitty config) do too. Plain bash degrades gracefully (ignores unknown CSI-u).
Picking kitty over ghostty / iTerm
All five allowlist values would fix the bug. Chose
kittybecause:TERM?.includes("kitty")still match.ghosttygets compared against>=1.2.0,iTerm.appgets color-depth-gated on<3.x. OurTERM_PROGRAM_VERSIONis the Superset host-service version, which doesn't play nicely with those comparisons.Note on why this PR has a messy commit history
Went down several wrong paths before finding the real cause (alt-screen gating, kitty-flag tracking,
:1event-type suffix injection). All of that renderer-side machinery got reverted; the only surviving change is the one-line env update. Happy to squash for merge if preferred.Test plan
Summary by CodeRabbit