Skip to content

Commit 9dce810

Browse files
committed
fix: use -tt for forced PTY and -o cat for journalctl color passthrough
Two fixes discovered during live sandbox testing: 1. SSH PTY: Changed --ssh-flag=-t to --ssh-flag=-tt. The single -t requires stdin to be a terminal, but subprocess.Popen doesn't have one. Double -tt forces PTY allocation regardless. 2. journalctl output format: The default --short format strips ANSI escape codes from stored messages. When colors are enabled, use -o cat which outputs raw message content preserving ANSI codes written by the --pretty devserver.
1 parent b1d7523 commit 9dce810

4 files changed

Lines changed: 20 additions & 9 deletions

File tree

devservices/commands/sandbox.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -928,10 +928,14 @@ def sandbox_logs(args: Namespace) -> None:
928928
# Build the remote command
929929
if service in SANDBOX_SYSTEMD_SERVICES:
930930
unit = SANDBOX_SYSTEMD_SERVICES[service]
931+
# Use -o cat when colors are enabled: journalctl's default --short format
932+
# strips ANSI escape codes from stored messages, but -o cat outputs raw
933+
# message content preserving any ANSI codes written by --pretty devserver.
934+
output_fmt = " -o cat" if use_color else ""
931935
if follow:
932-
remote_cmd = f"sudo journalctl -u {unit} -n {lines} -f"
936+
remote_cmd = f"sudo journalctl -u {unit} -n {lines}{output_fmt} -f"
933937
else:
934-
remote_cmd = f"sudo journalctl -u {unit} -n {lines} --no-pager"
938+
remote_cmd = f"sudo journalctl -u {unit} -n {lines}{output_fmt} --no-pager"
935939
else:
936940
# Docker container — find by partial name match
937941
follow_flag = "-f " if follow else ""

devservices/utils/sandbox.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,8 +297,9 @@ def ssh_stream(
297297
Unlike ssh_command() which captures output, this passes stdout/stderr
298298
directly through to the caller's terminal, suitable for follow/tail mode.
299299
300-
When tty=True, allocates a PTY on the remote side (--ssh-flag=-t) so that
301-
programs which detect a terminal (e.g. journalctl) emit colored output.
300+
When tty=True, forces PTY allocation on the remote side (--ssh-flag=-tt)
301+
so that programs which detect a terminal (e.g. journalctl) emit colored
302+
output. Uses -tt (double) to force allocation even without a local tty.
302303
"""
303304
cmd = [
304305
"gcloud",
@@ -310,7 +311,7 @@ def ssh_stream(
310311
"--tunnel-through-iap",
311312
]
312313
if tty:
313-
cmd.append("--ssh-flag=-t")
314+
cmd.append("--ssh-flag=-tt")
314315
cmd.append(f"--command={command}")
315316
try:
316317
return subprocess.Popen(cmd)

tests/commands/test_sandbox.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3128,6 +3128,7 @@ def test_sandbox_logs_follow_color(
31283128
assert call_kwargs["tty"] is True
31293129
remote_cmd = mock_ssh_stream.call_args[0][3]
31303130
assert ANSI_STRIP_PIPE not in remote_cmd
3131+
assert "-o cat" in remote_cmd
31313132
mock_proc.wait.assert_called()
31323133

31333134

@@ -3170,6 +3171,7 @@ def test_sandbox_logs_follow_no_color(
31703171
assert call_kwargs["tty"] is False
31713172
remote_cmd = mock_ssh_stream.call_args[0][3]
31723173
assert ANSI_STRIP_PIPE in remote_cmd
3174+
assert "-o cat" not in remote_cmd
31733175
mock_proc.wait.assert_called()
31743176

31753177

@@ -3212,6 +3214,7 @@ def test_sandbox_logs_no_follow_color(
32123214
assert call_kwargs["tty"] is True
32133215
remote_cmd = mock_ssh_stream.call_args[0][3]
32143216
assert ANSI_STRIP_PIPE not in remote_cmd
3217+
assert "-o cat" in remote_cmd
32153218
mock_proc.wait.assert_called()
32163219

32173220

@@ -3254,6 +3257,7 @@ def test_sandbox_logs_no_follow_no_color(
32543257
assert call_kwargs["tty"] is False
32553258
remote_cmd = mock_ssh_stream.call_args[0][3]
32563259
assert ANSI_STRIP_PIPE in remote_cmd
3260+
assert "-o cat" not in remote_cmd
32573261
mock_proc.wait.assert_called()
32583262

32593263

@@ -3299,6 +3303,7 @@ def test_sandbox_logs_auto_color_tty(
32993303
assert call_kwargs["tty"] is True
33003304
remote_cmd = mock_ssh_stream.call_args[0][3]
33013305
assert ANSI_STRIP_PIPE not in remote_cmd
3306+
assert "-o cat" in remote_cmd
33023307

33033308

33043309
@mock.patch("devservices.commands.sandbox.validate_sandbox_prerequisites")
@@ -3343,6 +3348,7 @@ def test_sandbox_logs_auto_color_pipe(
33433348
assert call_kwargs["tty"] is False
33443349
remote_cmd = mock_ssh_stream.call_args[0][3]
33453350
assert ANSI_STRIP_PIPE in remote_cmd
3351+
assert "-o cat" not in remote_cmd
33463352

33473353

33483354
@mock.patch("devservices.commands.sandbox.validate_sandbox_prerequisites")

tests/utils/test_sandbox.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -736,26 +736,26 @@ def test_ssh_command_gcloud_not_found(mock_run_gcloud: mock.Mock) -> None:
736736

737737
@mock.patch("subprocess.Popen")
738738
def test_ssh_stream_with_tty(mock_popen: mock.Mock) -> None:
739-
"""ssh_stream with tty=True adds --ssh-flag=-t to the command."""
739+
"""ssh_stream with tty=True adds --ssh-flag=-tt to the command."""
740740
mock_proc = mock.Mock()
741741
mock_popen.return_value = mock_proc
742742
result = ssh_stream(
743743
"sandbox-test", "my-project", "us-central1-a", "echo hello", tty=True
744744
)
745745
assert result is mock_proc
746746
call_args = mock_popen.call_args[0][0]
747-
assert "--ssh-flag=-t" in call_args
747+
assert "--ssh-flag=-tt" in call_args
748748

749749

750750
@mock.patch("subprocess.Popen")
751751
def test_ssh_stream_without_tty(mock_popen: mock.Mock) -> None:
752-
"""ssh_stream with tty=False (default) does NOT add --ssh-flag=-t."""
752+
"""ssh_stream with tty=False (default) does NOT add --ssh-flag=-tt."""
753753
mock_proc = mock.Mock()
754754
mock_popen.return_value = mock_proc
755755
result = ssh_stream("sandbox-test", "my-project", "us-central1-a", "echo hello")
756756
assert result is mock_proc
757757
call_args = mock_popen.call_args[0][0]
758-
assert "--ssh-flag=-t" not in call_args
758+
assert "--ssh-flag=-tt" not in call_args
759759

760760

761761
# --- check_api_enabled ---

0 commit comments

Comments
 (0)