Fix cmux themes ctrl navigation#4366
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThe ghostty submodule is bumped to a commit that adds Ctrl-N/Ctrl-P navigation to the cmux theme picker; docs and the pinned xcframework reference are updated, a checksum entry is added, and tests are extended to validate Ctrl-N and Ctrl-P picker scenarios. ChangesCtrl Navigation Support in Theme Picker
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 17✅ Passed checks (17 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 adds PTY regression coverage for Ctrl-N/Ctrl-P navigation in the bundled Ghostty theme picker and advances the Ghostty submodule to the commit that implements the fix. All related documentation, checksums, and submodule references are kept in sync.
Confidence Score: 5/5Safe to merge — changes are test additions, a submodule pointer bump, a checksum append, and documentation updates with no production Swift or runtime code touched. All four changed files are either test scaffolding, a submodule reference, a checksums registry, or documentation. The test logic correctly models the picker's alphabetical sort order, the Ctrl-N/Ctrl-P byte sequences are the standard ASCII control codes, and all previously stale SHA references in the docs have been updated to the new submodule head. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Test as test_bundled_ghostty_theme_picker_helper.sh
participant PTY as PTY (pty.fork)
participant Helper as ghostty +list-themes
participant Config as scenario_config.ghostty
Test->>PTY: "fork child with CMUX_THEME_PICKER_* env vars"
PTY->>Helper: execve ghostty +list-themes
Helper-->>PTY: renders picker UI (stdout)
PTY-->>Test: output includes "Enter apply"
Test->>PTY: "write scripted_input (\x0e=Ctrl-N, \x10=Ctrl-P, \r=Enter)"
Helper->>Config: "writes theme = light:X,dark:X"
Helper-->>PTY: exits 0
PTY-->>Test: child exits
Test->>Config: assert_theme_written(expected_theme)
Test->>Test: bash loop validates start/end markers + theme line
Reviews (4): Last reviewed commit: "fix: support ctrl navigation in cmux the..." | Re-trigger Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
lawrencecchen
force-pushed
the
issue-themes-broken-ctrl-np
branch
from
10c21af to
6e0f22c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ghostty-fork.md (1)
80-91:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the “current pinned head/release tag” sections to match
176bd550f.After adding
176bd550fhere, the document still says the current pin isff6e1260d(Line 16) and still referencesxcframework-ff6e1260d...as the current published/pinned archive (Line 24, Lines 216-219). Please update those sections so this doc has a single authoritative current head.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/ghostty-fork.md` around lines 80 - 91, Update the document's "current pinned head/release tag" references from ff6e1260d to 176bd550f: find every occurrence of the old tag (e.g., the header that states the current pinned head, any mentions like "xcframework-ff6e1260d..." and the published/pinned archive references) and replace them with 176bd550f so the document consistently shows the new pinned head; verify the commit id is updated in the summary and any archive filenames or release-tag strings throughout the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@docs/ghostty-fork.md`:
- Around line 80-91: Update the document's "current pinned head/release tag"
references from ff6e1260d to 176bd550f: find every occurrence of the old tag
(e.g., the header that states the current pinned head, any mentions like
"xcframework-ff6e1260d..." and the published/pinned archive references) and
replace them with 176bd550f so the document consistently shows the new pinned
head; verify the commit id is updated in the summary and any archive filenames
or release-tag strings throughout the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9d60877-19b1-402e-bf0f-8ca676468830
📒 Files selected for processing (4)
docs/ghostty-fork.mdghosttyscripts/ghosttykit-checksums.txttests/test_bundled_ghostty_theme_picker_helper.sh
lawrencecchen
force-pushed
the
issue-themes-broken-ctrl-np
branch
from
6e0f22c to
36fd55d
Compare
lawrencecchen
force-pushed
the
issue-themes-broken-ctrl-np
branch
from
36fd55d to
7b0d567
Compare
|
Superseded by the targeted stacked follow-up PR: #4404. |
1 participant
Summary
What changed:
cmux themesCtrl-N and Ctrl-P navigation.176bd550f6fedd29e85cd92470e5dfadf295ebf7.docs/ghostty-fork.md.Why:
cmux themesacceptedjandk, but Ctrl-N and Ctrl-P did not move selection, so Enter could apply the wrong highlighted theme.theme = light:X,dark:Xline so this cannot silently pass by only checking that a config file exists.Testing
tests/test_bundled_ghostty_theme_picker_helper.shfailed before the fix with Ctrl-N still applying the first bundled theme, expected12-bit Rainbow, got0x96f.tests/test_bundled_ghostty_theme_picker_helper.shpassed after the fix against the rebuilt tagged app.CMUX_APP_PATH="$HOME/Library/Developer/Xcode/DerivedData/cmux-themesfix/Build/Products/Debug/cmux DEV themesfix.app" tests/test_bundled_ghostty_theme_picker_helper.sh../tests/test_ci_ghosttykit_checksum_present.sh.GHOSTTY_SHA=176bd550f6fedd29e85cd92470e5dfadf295ebf7 GHOSTTYKIT_OUTPUT_DIR=/tmp/cmux-ghosttykit-download-check ./scripts/download-prebuilt-ghosttykit.sh../tests/test_ci_ghosttykit_checksum_verification.sh../scripts/reload.sh --tag themesfix.96e89a8d5 test: cover cmux theme picker ctrl navigation.7b0d5675c fix: support ctrl navigation in cmux themes.Proof
/Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-themes-broken-ctrl-np/auto-issue/20260519-030016/before/recording.mov.theme = light:0x96f,dark:0x96fand exits 1./Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-themes-broken-ctrl-np/auto-issue/20260519-030016/before/frames/02.pngand/Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-themes-broken-ctrl-np/auto-issue/20260519-030016/before/frames/tail.png./Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-themes-broken-ctrl-np/auto-issue/20260519-030016/after/recording.mov./Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-themes-broken-ctrl-np/auto-issue/20260519-030016/after/frames/02.pngand/Users/lawrence/fun/cmuxterm-hq/cmux-assets/issue-themes-broken-ctrl-np/auto-issue/20260519-030016/after/frames/tail.png.Artifacts
Notes
Checklist