Skip to content

Conversation

@jif-oai
Copy link
Collaborator

@jif-oai jif-oai commented Jan 13, 2026

No description provided.

@jif-oai
Copy link
Collaborator Author

jif-oai commented Jan 13, 2026

@codex review

Copy link
Contributor

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

Choose a reason for hiding this comment

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

💡 Codex Review

// Long‑lived command: persist the process so write_stdin can reuse
// it, and register a background watcher that will emit
// ExecCommandEnd when the PTY eventually exits (even if no further
// tool calls are made).
self.store_process(

P2 Badge Preserve waiting status events for non-TUI clients

Removing the waiting-status emission here means long‑lived unified exec sessions no longer produce BackgroundEvent notifications. The new waiting UI is driven by TerminalInteractionEvent in tui, but tui2 still leaves on_terminal_interaction as TODO and the exec CLI ignores EventMsg::TerminalInteraction while only printing BackgroundEvent (see codex-rs/tui2/src/chatwidget.rs and codex-rs/exec/src/event_processor_with_human_output.rs). As a result, when a background terminal is waiting (empty‑stdin polls), those clients never surface any “waiting” feedback. Consider keeping the background event (or emitting an alternative status update) for non‑TUI consumers.

ℹ️ 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 on lines -463 to -468
session
.send_event(
turn.as_ref(),
EventMsg::BackgroundEvent(BackgroundEventEvent { message }),
)
.await;
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we want to update the app/VSCE and web?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep probably, it's why I pinged you

Copy link
Collaborator

Choose a reason for hiding this comment

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

For VSCE and app we do not use this for any rendering yet.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's come up with something, I bet we want to use it at some point.

cc: @gabriel-openai @owenlin0

Copy link
Collaborator

@shijie-oai shijie-oai left a comment

Choose a reason for hiding this comment

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

LGTM from VCSE/app side.

@jif-oai jif-oai merged commit 7532f34 into main Jan 14, 2026
38 of 44 checks passed
@jif-oai jif-oai deleted the jif/double-waiting branch January 14, 2026 09:52
@github-actions github-actions bot locked and limited conversation to collaborators Jan 14, 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.

4 participants