Skip to content

Fix TTY detach handling and keep Auto Drive completion interactive#561

Open
cbusillo wants to merge 8 commits intojust-every:mainfrom
cbusillo:fix/exec-detach-noninteractive-clean-pr
Open

Fix TTY detach handling and keep Auto Drive completion interactive#561
cbusillo wants to merge 8 commits intojust-every:mainfrom
cbusillo:fix/exec-detach-noninteractive-clean-pr

Conversation

@cbusillo
Copy link

@cbusillo cbusillo commented Mar 10, 2026

Why

This branch fixes a class of terminal-lifecycle bugs where noninteractive work was still too coupled to the controlling tty.

In practice, that showed up as fragile detach/reattach behavior, cases where the TUI could appear unresponsive after terminal handoff or session transitions, one concrete failure mode where a long-lived shell-tool descendant launched via nohup ... & could later take foreground ownership of the same terminal, and a related recovery failure where the tty could stay pointed at a dead foreground process group after that descendant exited. Background helpers and read-only agent runs do not need interactive stdin, and redirected shell-tool commands should not stay in the TUI's controlling session at all.

The foreground TUI also should not attempt stdin reads while it is out of the foreground process group. If the tty foreground group is already dead, the TUI should reclaim it instead of sleeping forever.

While validating the patched binary locally, I also hit a startup-only debug panic: a prelude path inserted an MCP-connecting BackgroundEvent through the generic prelude helper instead of the dedicated background-event path. This change routes startup background preludes through background tagging so debug builds no longer panic on launch.

During the same investigation, I found a separate but user-visible Auto Drive bug: after a run completed, the UI could prioritize the next goal-entry prompt over the completion summary. That made the end state misleading and caused Esc to target the wrong surface first. This PR keeps the completion summary visible until it is explicitly dismissed.

What Changed

  • detach background helper commands from the controlling tty
  • detach read-only agent stdin from the tty
  • start redirected shell-tool launches in a new Unix session instead of only a new process group
  • skip TUI stdin reads while the app is not in the foreground process group
  • reclaim the tty foreground process group when it points at a dead process group instead of waiting indefinitely
  • route startup background preludes through background-event tagging to avoid debug-assert panics
  • keep the Auto Drive completion summary visible until dismissed with Esc before showing the next goal prompt

Validation

  • cargo test -p code-core spawn::tests -- --nocapture
  • cargo test -p code-tui unix_tty_tests -- --nocapture
  • cargo test -p code-tui startup_background_prelude_uses_background_tagging -- --nocapture
  • cargo test -p code-tui completed_auto_drive_prefers_summary_before_next_goal_prompt -- --nocapture
  • PROFILE=release-prod ./build-fast.sh

@cbusillo cbusillo marked this pull request as ready for review March 10, 2026 21:48
Copy link

@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

Here are some automated review suggestions for this pull request.

Reviewed commit: 672368e32f

ℹ️ 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 +155 to +156
if self.auto_state.last_run_summary.is_some() {
return EscRoute::new(EscIntent::AutoDismissSummary, true, false);

Choose a reason for hiding this comment

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

P2 Badge Preserve goal draft when handling Esc on completion summary

Routing Esc to AutoDismissSummary before the goal-entry intents causes a draft-loss path after Auto Drive completes: the phase is still AwaitingGoalEntry, so users can start typing the next goal, but pressing Esc now dismisses the summary first and triggers auto_clear_summary_panel()auto_rebuild_live_ring()auto_show_goal_entry_panel(), which clears the composer and drops the typed draft. This regresses the prior draft-preserving goal-entry escape flow for users who begin composing before dismissing the summary.

Useful? React with 👍 / 👎.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in b5bd6c293.

You were right: dismissing the completion summary could rebuild goal entry by calling auto_show_goal_entry_panel(), which cleared the composer and dropped a drafted next goal. The summary-dismiss path now restores goal entry while preserving any non-empty draft, and I added a regression test covering that flow.

Validation:

  • cargo test -p code-tui dismissing_completed_summary_preserves_typed_next_goal -- --nocapture
  • cargo test -p code-tui completed_auto_drive_prefers_summary_before_next_goal_prompt -- --nocapture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant