Skip to content

test(shell): cover tty controlling terminal#2414

Merged
Hmbown merged 2 commits into
mainfrom
codex/harvest-2408-tty-controlling-terminal-test
May 31, 2026
Merged

test(shell): cover tty controlling terminal#2414
Hmbown merged 2 commits into
mainfrom
codex/harvest-2408-tty-controlling-terminal-test

Conversation

@Hmbown
Copy link
Copy Markdown
Owner

@Hmbown Hmbown commented May 31, 2026

Summary

  • harvests test(shell): cover tty controlling terminal #2408 with credit to @axobase001
  • adds a Unix regression test for tty=true background shell jobs opening /dev/tty
  • exercises the same shell-manager path used by task_shell_start with DangerFullAccess, so controlling-terminal setup failures are caught in CI
  • gives the get_output wait window extra margin so the test is less brittle on loaded runners

Partially addresses #2372

Validation

  • cargo fmt --all -- --check
  • git diff --check
  • CARGO_TARGET_DIR=/Volumes/VIXinSSD/codewhale-pr2408-target cargo test -p codewhale-tui --bin codewhale-tui background_tty_command_has_controlling_terminal --locked -- --nocapture
  • CARGO_TARGET_DIR=/Volumes/VIXinSSD/codewhale-pr2408-check-target cargo check -p codewhale-tui --locked

Copilot AI review requested due to automatic review settings May 31, 2026 09:35
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

Hmbown has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds a new Unix-specific test, background_tty_command_has_controlling_terminal, to crates/tui/src/tools/shell/tests.rs. The test verifies that background commands executed via the shell manager have access to a controlling terminal by opening /dev/tty and asserting that the command completes successfully with the expected output. There are no review comments, and I have no additional feedback to provide.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@Hmbown Hmbown merged commit 6e84773 into main May 31, 2026
20 of 23 checks passed
@Hmbown Hmbown deleted the codex/harvest-2408-tty-controlling-terminal-test branch May 31, 2026 09:47
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.

3 participants