Fix Korean IME jamo leak during composition

[lawrencecchen]

Summary

• add a regression that catches composing Korean key events leaking text into Ghostty
• keep composing keydowns on the nil-text path until the IME commits or cancels
• preserve the existing Korean Return commit behavior

Testing

• CMUX_SKIP_ZIG_BUILD=1 xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux-unit -destination 'platform=macOS' -derivedDataPath /tmp/cmux-issue-2519-korean-ime-tests test -only-testing:cmuxTests/KoreanIMEMarkedTextLeakRegressionTests -only-testing:cmuxTests/KoreanIMEReturnCommitRegressionTests

Fixes #2519

---

Summary by cubic

Prevents Korean IME jamo from leaking into the terminal during composition by suppressing fallback text until the IME commits or cancels. Fixes #2519.

• Bug Fixes

  • Suppress fallback text while marked text is active; composing keydowns stay on the nil-text path until commit or cancel.
  • Preserve Korean Return-to-commit behavior and add regression tests for jamo leak and Return commit.

• Refactors

  • In DEBUG, send keys via sendTimedGhosttyKey (release uses sendGhosttyKey) and rename timing metric paths to "*.total".

^{Written for commit 1968d40. Summary will update on new commits.}

Summary by CodeRabbit

• Bug Fixes

  • Fixed Korean IME text composition to prevent unintended character leaks while actively composing marked text, improving reliability during multi-stage input.

• Tests

  • Added a regression test to verify correct Korean IME input handling while marked text is active.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
cmux	Ready	Preview, Comment	Apr 2, 2026 10:34am

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 656e1aa9-4be0-4455-b7d0-ec4a53e5edae

📥 Commits

Reviewing files that changed from the base of the PR and between 9b416af and 1968d40.

📒 Files selected for processing (1)

• cmuxTests/CJKIMEInputTests.swift

---

📝 Walkthrough

Walkthrough

Added DEBUG-only timing instrumentation around Ghostty key sends, prevented fallback text sends when a key event is marked composing, and added a Korean IME regression test ensuring jamo are not leaked while marked text is active.

Changes

Cohort / File(s)	Summary
IME Key Event Instrumentation & Composing Suppression Sources/GhosttyTerminalView.swift	Replaced direct ghostty_surface_key(...) calls with a DEBUG timing wrapper (sendTimedGhosttyKey(..., path: ...)) and release sendGhosttyKey(...). Added suppressComposingFallbackText = keyEvent.composing and gated fallback shouldSendText(text) to require !suppressComposingFallbackText. Updated log timing paths (*.total).
Korean IME Regression Test cmuxTests/CJKIMEInputTests.swift	Added @MainActor test class KoreanIMEMarkedTextLeakRegressionTests and test testKeyDownDoesNotLeakJamoWhileMarkedTextIsActive() which boots a TerminalSurface+GhosttyNSView, sets Korean marked text, installs IME swizzle and debug hooks, sends a jamo keyDown, and asserts the key event remains composing with no committed text and marked text still present.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• Fix Ctrl+H deleting committed text instead of composing text during IME input #790: Modifies GhosttyTerminalView.swift key handling to avoid bypassing IME composition—closely related to the composing-suppression changes.
• fix(terminal): execute Return after Korean IME commit #1671: Changes GhosttyNSView IME/key-down handling and adds Korean IME tests—directly related to the new regression test and key-send behavior.
• Fix Shift+Space IME toggle inserting a space (#641) #670: Adds IME fallback-text suppression logic in the same send path; shares the suppression-pattern and timing of text-send gating.

Poem

🐰 I hop on keys where jamo hide and play,
I time each press in DEBUG light of day,
When composing blooms I hush the fallback voice,
No split syllables — the letters reunite, rejoice! 🥕✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly describes the main fix: preventing Korean IME jamo from leaking during composition, which matches the core changes in the codebase.
Description check	✅ Passed	The PR description covers the key changes and testing approach, but omits several required template sections: Demo Video, Review Trigger block, and checklist items.
Linked Issues check	✅ Passed	The code changes directly address issue #2519 by suppressing fallback text during IME composition and adding regression tests to prevent jamo leakage into the terminal.
Out of Scope Changes check	✅ Passed	All changes are scoped to the Korean IME jamo leak fix: instrumenting key event handling, adding composition state checks, and test coverage for the regression.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-2519-korean-ime

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[greptile-apps]

Greptile Summary

This PR fixes a Korean IME (Hangul) jamo leak during composition in the terminal surface. When composing with the Korean 2-Set IME, the keyDown fallback text path was unconditionally forwarding individual jamo characters (e.g. ㄴ) as text to Ghostty even while marked text was still active, causing double-input. The fix adds a suppressComposingFallbackText guard derived from keyEvent.composing, routing composing keydowns through the nil-text path so Ghostty receives the structural key event without committed text.

Key changes:

• GhosttyTerminalView.swift: Adds suppressComposingFallbackText = keyEvent.composing to the fallback text condition. Also refactors the two ghostty_surface_key direct call sites in the text-send paths into sendTimedGhosttyKey/sendGhosttyKey helpers (DEBUG/RELEASE split), and renames the outer timing log paths with a .total suffix to avoid name collision with the new per-send timer.
• CJKIMEInputTests.swift: Adds KoreanIMEMarkedTextLeakRegressionTests with a test that installs the cjkIMEInterpretKeyEventsHook swizzle to suppress insertText, sets marked text on the view, fires a jamo keydown, and asserts that the forwarded key event has composing=true and text=nil while hasMarkedText() remains true.

Confidence Score: 4/5

Safe to merge; the fix is narrowly scoped to the composing fallback path and is backed by a regression test.

The logic change is minimal and well-isolated: one new boolean (suppressComposingFallbackText = keyEvent.composing) gates the text-send branch, mirroring the pre-existing suppressShiftSpaceFallbackText pattern. The regression test correctly exercises the new code path. The only deducted point is a minor naming clarity issue in the test that could cause confusion during future triage.

No files require special attention beyond the minor test variable naming noted in the comment.

Important Files Changed

Filename	Overview
Sources/GhosttyTerminalView.swift	Adds suppressComposingFallbackText guard to prevent Hangul jamo from leaking as committed text; refactors direct ghostty_surface_key calls to sendGhosttyKey/sendTimedGhosttyKey; renames timing log path to add .total suffix.
cmuxTests/CJKIMEInputTests.swift	Adds KoreanIMEMarkedTextLeakRegressionTests class with a test that verifies composing keydowns produce a composing=true, text=nil event and do not leak jamo text; minor blank-line removal.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[keyDown event received] --> B{accumulatedText empty?}
    B -- No --> C[Send accumulated insertText results\ncomposing=false, text=committed string]
    B -- Yes --> D[Compute suppressors]
    D --> E{textForKeyEvent returns text?}
    E -- No --> F[Send key\ntext=nil, consumed_mods=NONE]
    E -- Yes --> G{shouldSendText AND\n!suppressShiftSpace AND\n!suppressComposing?}
    G -- No\n'composing=true OR shift+space OR non-text key' --> F
    G -- Yes --> H[Send key\ntext=committed jamo/char]
    C --> I[ghostty_surface_key]
    F --> I
    H --> I

Loading

_{Reviews (1): Last reviewed commit: "fix: suppress fallback text during IME c..." | Re-trigger Greptile}

[greptile-apps]

Confusing variable name and guard failure message

leakedEvent is named as if capturing a bad/leaked event, but it actually holds the forwarded composing key event that is expected to arrive (the "leak" being tested is text ≠ nil, not the event itself). The XCTFail message at the guard makes this especially easy to misread: if the test fails here a developer sees "Expected terminal key event for composing Hangul input" and may think the intent was not to send an event, when in fact the guard asserts the opposite.

Consider renaming to make the intent clearer:

Suggested change

	guard let leakedEvent else {
	XCTFail("Expected terminal key event for composing Hangul input")
	return
	}
	guard let capturedEvent = leakedEvent else {
	XCTFail(
	"Expected a composing key event to be forwarded to Ghostty (with text=nil); no event was received"
	)
	return
	}

And update the subsequent leakedEvent references to capturedEvent.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

[lawrencecchen]

Renamed the test variable to capturedEvent and clarified the failure message so the intent is explicit: Ghostty should still receive a composing key event, just with text=nil.

— Claude Code

[cubic-dev-ai]

No issues found across 2 files