Skip to content

Fix stale background terminal poll events#23231

Merged
etraut-openai merged 2 commits into
mainfrom
etraut/fix-stale-terminal-poll
May 20, 2026
Merged

Fix stale background terminal poll events#23231
etraut-openai merged 2 commits into
mainfrom
etraut/fix-stale-terminal-poll

Conversation

@etraut-openai
Copy link
Copy Markdown
Collaborator

Why

Issue #23214 reports /ps showing no background terminals while the status line still says it is waiting for a background terminal. The race is in core: write_stdin can poll a process that exits before the response returns. The process manager correctly returns process_id: None, but the handler still emitted a TerminalInteraction event using the requested session id, causing clients to believe a dead process was still being polled.

Fixes #23214.

What changed

  • Suppress TerminalInteraction events for empty write_stdin polls once response.process_id is None.
  • Continue emitting interactions for non-empty stdin, even if that input causes the process to exit before the response returns.
  • Extend the unified exec integration test to assert completed empty polls do not emit terminal interactions.

Verification

  • cargo test -p codex-core --test all unified_exec_emits_one_begin_and_one_end_event
  • cargo test -p codex-core --test all unified_exec_emits_terminal_interaction_for_write_stdin

cargo test -p codex-core currently aborts in unrelated agent::control::tests::resume_agent_from_rollout_uses_edge_data_when_descendant_metadata_source_is_stale with a reproducible stack overflow.

@etraut-openai etraut-openai requested a review from a team as a code owner May 18, 2026 03:05
// still a live process for the UI to wait on. Non-empty stdin is a real
// terminal interaction and should remain visible even if it completes
// the process before the response returns.
if !args.chars.is_empty() || response.process_id.is_some() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This still seems racy for the adjacent ordering: if the process is alive when write_stdin builds the response but exits before this event is delivered, the exit watcher can clear the process first and then this empty interaction sets the stale “Waiting for background terminal” status again

@etraut-openai etraut-openai merged commit e43a2e2 into main May 20, 2026
31 checks passed
@etraut-openai etraut-openai deleted the etraut/fix-stale-terminal-poll branch May 20, 2026 03:48
@github-actions github-actions Bot locked and limited conversation to collaborators May 20, 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.

/ps not working during a goal

2 participants