Remove Zellij TUI workarounds#22214
Conversation
![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: 8ed2541bce
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/terminal-detection/src/lib.rs
Line 170 in ce30099
Preserve the public Zellij detection helper
Removing this public method is a breaking API change for users of codex-terminal-detection: existing callers that rely on TerminalInfo::is_zellij() now fail to compile even though the underlying multiplexer data still exists. If the workaround is gone, keeping this helper as a stable convenience avoids breaking downstream code.
codex/codex-rs/terminal-detection/src/lib.rs
Line 173 in ce30099
Preserve the public Zellij detection helper
Removing this public method is a breaking API change for users of codex-terminal-detection: existing callers that rely on TerminalInfo::is_zellij() now fail to compile even though the underlying multiplexer data still exists. If the workaround is gone, keeping this helper as a stable convenience avoids breaking downstream code.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| if area != terminal.viewport_area { | ||
| // TODO(nornagon): probably this could be collapsed with the clear + set_viewport_area above. | ||
| terminal.clear()?; |
There was a problem hiding this comment.
Calling terminal.clear() before set_viewport_area(area) regresses the startup/first-resize case where the old viewport is empty: Terminal::clear() returns immediately for an empty viewport, so stale shell cells inside the newly created inline viewport are not cleared. The removed clear_for_viewport_change test covered this exact scenario.
Useful? React with 👍 / 👎.
| pub fn insert_history_lines_with_wrap_policy<B>( | ||
| terminal: &mut crate::custom_terminal::Terminal<B>, | ||
| lines: Vec<Line>, | ||
| mode: InsertHistoryMode, | ||
| wrap_policy: HistoryLineWrapPolicy, | ||
| ) -> io::Result<()> |
There was a problem hiding this comment.
The change routes every Zellij session through the standard DECSTBM history path. The commit rationale only establishes that Zellij 0.44.1 works and says older Zellij broke on these sequences; without detecting or gating by Zellij version, users on older Zellij lose the scrollback/composer correctness the workaround was added for.
Useful? React with 👍 / 👎.
fcoury-oai
changed the title
1 participant
Why
We added Zellij-specific TUI workarounds because older Zellij behavior did not work with Codex's normal terminal model:
tui.alternate_screen = "auto"disable alternate screen in Zellij so transcript history stayed available.This PR removes both workarounds because the latest Zellij release tested locally (
zellij 0.44.1) works correctly with Codex's standard TUI behavior: normal alternate-screen handling, redraw, and history insertion.What Changed
InsertHistoryMode::Zellijpath and the Zellij-only newline scrollback insertion behavior.is_zellijstate from the TUI and composer.TerminalInfo::is_zellij()convenience method that only served this workaround.tui.alternate_screen = "auto"to use alternate screen for Zellij too;--no-alt-screenandtui.alternate_screen = "never"still preserve the inline mode escape hatch.tui.alternate_screen.How to Test
Manual smoke path used with
zellij 0.44.1:0.44.1session with default config.--no-alt-screenor-c tui.alternate_screen=neverif you want to verify the inline fallback still works.Targeted tests:
just write-config-schemajust fmtjust fix -p codex-tuicargo test -p codex-terminal-detectioncargo test -p codex-tui alternate_screen_auto_uses_alt_screenAttempted but did not complete locally:
cargo test -p codex-tuibuilt and ran the new test successfully, then failed later on unrelated local failures instatus_permissions_full_disk_managed_*and a stack overflow intests::fork_last_filters_latest_session_by_cwd_unless_show_all.Documentation
No developers.openai.com Codex documentation update is needed for this revert.