Skip to content

fix(server): respect tty flag in ExecSandboxInteractive#2316

Closed
letv1nnn wants to merge 2 commits into
NVIDIA:mainfrom
letv1nnn:exec-sandbox-interactive-tty
Closed

fix(server): respect tty flag in ExecSandboxInteractive#2316
letv1nnn wants to merge 2 commits into
NVIDIA:mainfrom
letv1nnn:exec-sandbox-interactive-tty

Conversation

@letv1nnn

@letv1nnn letv1nnn commented Jul 16, 2026

Copy link
Copy Markdown

Summary

ExecSandboxInteractive accepted tty=false in the start request but unconditionally allocated a PTY over SSH, causing processes to always observe terminal-backed file descriptors. This PR threads the tty flag through the interactive exec call chain and guards request_pty and window_change behind it.

Related Issue

Fixes #2228

Changes

  • Thread tty: bool through stream_interactive_exec_over_relay and run_interactive_exec_with_russh
  • Guard request_pty and window_change behind if tty
  • Add unit test for tty=false validation

Reproduction

Used the reproduction script from #2228 to verify behavior before and after the fix.

Before:
image

After:
image

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

letv1nnn added 2 commits July 16, 2026 14:51
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
Signed-off-by: Artem Lytvyn <alytvyn@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 16, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@johntmyers

Copy link
Copy Markdown
Collaborator

dupe of #2315

@johntmyers johntmyers closed this Jul 17, 2026
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.

[Bug]: ExecSandboxInteractive ignores tty=false and always allocates a PTY

2 participants