feat(tui): remove Zellij TUI workarounds#22214
Conversation
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
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
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".
6140c2b to
da19ebc
Compare
Treat tui.alternate_screen = "auto" the same as normal alternate-screen mode, leaving never and --no-alt-screen as the explicit opt-outs. Also trim the config docs and schema so they no longer describe Zellij-specific detection.
etraut-openai
left a comment
There was a problem hiding this comment.
Looks good from a config standpoint. Doesn't change any config logic; just updates some description strings.
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.