Skip to content

fix(tui): bound startup terminal probes#20654

Merged
fcoury-oai merged 2 commits into
mainfrom
codex/bounded-terminal-startup-probes
May 2, 2026
Merged

fix(tui): bound startup terminal probes#20654
fcoury-oai merged 2 commits into
mainfrom
codex/bounded-terminal-startup-probes

Conversation

@fcoury-oai

Copy link
Copy Markdown
Contributor

Summary

Bound TUI startup terminal response probes so unsupported terminals cannot stall startup for multiple seconds.

This replaces the Unix startup uses of crossterm's blocking response probes with short /dev/tty probes that use nonblocking reads and poll with a 100ms timeout. It covers the initial cursor-position query, keyboard enhancement support detection, and OSC 10/11 default-color detection. The default-color probe uses one shared deadline for foreground and background instead of allowing two independent full waits.

The diagnostic mode/trace env vars from the investigation branch are intentionally not included. The shipped behavior is simply bounded probing by default, while non-Unix keeps the existing crossterm fallback path.

Details

  • Add a private terminal_probe module for bounded Unix terminal probes and response parsers.
  • Let custom_terminal::Terminal accept a caller-provided initial cursor position so startup can compute it before constructing the terminal.
  • Use bounded cursor, keyboard enhancement, and default-color probes on Unix startup.
  • Preserve default-color cache behavior so a failed attempted query does not retry forever.

Validation

  • cd codex-rs && just fmt
  • cd codex-rs && cargo test -p codex-tui terminal_probe
  • cd codex-rs && just fix -p codex-tui
  • cd codex-rs && just argument-comment-lint
  • git diff --check
  • git diff --cached --check

cd codex-rs && cargo test -p codex-tui still aborts on the pre-existing local stack overflow in app::tests::discard_side_thread_keeps_local_state_when_server_close_fails; I reproduced that same focused failure on main before this PR work, so it is not introduced by this change.

Manual validation in the VM showed the original crossterm path taking about 2s per unanswered probe, while bounded probing returned in about 100ms per probe.

@fcoury-oai fcoury-oai changed the title [codex] Bound TUI startup terminal probes fix(tui): bound startup terminal probes May 1, 2026
@fcoury-oai fcoury-oai marked this pull request as ready for review May 1, 2026 18:15
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from a026d35 to 9bc5229 Compare May 1, 2026 18:28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a026d3585b

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_probe.rs Outdated
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 9bc5229 to 7575586 Compare May 1, 2026 18:39
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 75755866c0

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_probe.rs Outdated
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 7575586 to 9d2f3b5 Compare May 1, 2026 19:00
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d2f3b5e14

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_probe.rs
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d2f3b5e14

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_probe.rs Outdated
Comment thread codex-rs/tui/src/terminal_probe.rs
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 9d2f3b5 to 643a3fe Compare May 1, 2026 19:36
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 643a3feca1

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_probe.rs Outdated
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch 2 times, most recently from bbe7dea to 1666069 Compare May 1, 2026 20:01
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1666069646

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_palette.rs Outdated
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 1666069 to 6954e00 Compare May 1, 2026 20:13
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6954e00777

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_probe.rs Outdated
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 6954e00 to 1eb06c0 Compare May 1, 2026 20:35
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1eb06c07f7

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_palette.rs
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 1eb06c0 to 6d93940 Compare May 1, 2026 20:52
@fcoury-oai

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6d9394071b

ℹ️ 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".

Comment thread codex-rs/tui/src/terminal_probe.rs Outdated
Comment thread codex-rs/tui/src/terminal_probe.rs
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 6d93940 to 29fb2be Compare May 1, 2026 21:07

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. 👍

ℹ️ 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".

@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from 29fb2be to dc467e7 Compare May 1, 2026 21:31
fcoury-oai added 2 commits May 1, 2026 21:59
Prefer duplicated stdin/stdout for bounded startup probes so terminal replies are read from the same stream as crossterm. This restores default color detection in terminals like Ghostty where opening /dev/tty separately did not see probe responses.

Query OSC 10 and OSC 11 together under one shared deadline. Terminals that do not support default-color replies, such as PuTTY, now pay one bounded wait instead of falling through to crossterm's multi-second default-color timeout.
@fcoury-oai fcoury-oai force-pushed the codex/bounded-terminal-startup-probes branch from cb18c72 to 47e91dc Compare May 2, 2026 01:00
@fcoury-oai fcoury-oai enabled auto-merge (squash) May 2, 2026 01:12
@fcoury-oai fcoury-oai merged commit 127434c into main May 2, 2026
26 checks passed
@fcoury-oai fcoury-oai deleted the codex/bounded-terminal-startup-probes branch May 2, 2026 01:20
@github-actions github-actions Bot locked and limited conversation to collaborators May 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants