Fix Shift+Enter not working in macos-container TUI #246
@@ -4,11 +4,16 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import os
|
import os
|
||||||
import subprocess
|
import subprocess
|
||||||
|
import sys
|
||||||
from typing import Callable, cast
|
from typing import Callable, cast
|
||||||
|
|
||||||
from ...agent_provider import PromptMode, prompt_args
|
from ...agent_provider import PromptMode, prompt_args
|
||||||
from .. import Bottle, ExecResult
|
from .. import Bottle, ExecResult
|
||||||
from ..terminal import exec_shell_script
|
from ..terminal import exec_shell_script
|
||||||
|
from . import pty_forward as _pty_forward
|
||||||
|
|
||||||
|
|
||||||
|
_PTY_FORWARD_SCRIPT = _pty_forward.__file__
|
||||||
|
|
||||||
|
|
||||||
class MacosContainerBottle(Bottle):
|
class MacosContainerBottle(Bottle):
|
||||||
@@ -45,18 +50,25 @@ class MacosContainerBottle(Bottle):
|
|||||||
argv=full_argv,
|
argv=full_argv,
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
cmd = ["container", "exec"]
|
container_exec = ["container", "exec"]
|
||||||
if tty:
|
if tty:
|
||||||
cmd.extend(["--interactive", "--tty"])
|
container_exec.extend(["--interactive", "--tty"])
|
||||||
# Forward TERM so Claude Code can enable modifier-key protocols
|
# Forward TERM so Claude Code can enable modifier-key protocols
|
||||||
# (e.g. modifyOtherKeys). Without it the inner PTY session has no
|
# (e.g. modifyOtherKeys / kitty protocol). Without it the inner
|
||||||
# TERM and Shift+Enter is indistinguishable from plain Enter.
|
# PTY session has no terminal-type context and Shift+Enter is
|
||||||
|
# indistinguishable from plain Enter.
|
||||||
term = os.environ.get("TERM", "xterm-256color")
|
term = os.environ.get("TERM", "xterm-256color")
|
||||||
cmd.extend(["--env", f"TERM={term}"])
|
container_exec.extend(["--env", f"TERM={term}"])
|
||||||
if self.agent_workdir and self.agent_workdir != "/home/node":
|
if self.agent_workdir and self.agent_workdir != "/home/node":
|
||||||
cmd.extend(["--workdir", self.agent_workdir])
|
container_exec.extend(["--workdir", self.agent_workdir])
|
||||||
cmd.extend([self.name, self.agent_command, *full_argv])
|
container_exec.extend([self.name, self.agent_command, *full_argv])
|
||||||
return cmd
|
if tty:
|
||||||
|
# Wrap with the raw-mode forwarder: container exec does not put
|
||||||
|
# the host terminal into raw mode itself, so the line discipline
|
||||||
|
# buffers modifier-key sequences until CR. The wrapper sets raw
|
||||||
|
# mode before exec and restores it on exit.
|
||||||
|
return [sys.executable, _PTY_FORWARD_SCRIPT, "--", *container_exec]
|
||||||
|
return container_exec
|
||||||
|
|
||||||
def exec_agent(self, argv: list[str], *, tty: bool = True) -> int:
|
def exec_agent(self, argv: list[str], *, tty: bool = True) -> int:
|
||||||
agent_argv = self.agent_argv(argv, tty=tty)
|
agent_argv = self.agent_argv(argv, tty=tty)
|
||||||
|
|||||||
@@ -0,0 +1,60 @@
|
|||||||
|
"""Host-side raw-mode wrapper for `container exec --interactive --tty`.
|
||||||
|
|
||||||
|
Apple's `container exec --interactive --tty` does not set the host terminal to
|
||||||
|
raw mode before starting its I/O relay. Without raw mode the kernel line
|
||||||
|
discipline buffers modifier-key escape sequences (e.g. Shift+Enter in
|
||||||
|
modifyOtherKeys mode produces \\x1b[13;2~) until a carriage-return arrives, so
|
||||||
|
they never reach Claude Code inside the container.
|
||||||
|
|
||||||
|
This module sets the host terminal to raw mode, spawns the inner argv (the
|
||||||
|
container exec command), and restores the original terminal attributes on
|
||||||
|
exit. When stdin is not a TTY (piped invocations, CI) it falls through to a
|
||||||
|
bare subprocess.run so callers do not need to special-case non-interactive
|
||||||
|
contexts.
|
||||||
|
|
||||||
|
Usage (the `--` separator is the API contract — everything after it is the
|
||||||
|
inner command):
|
||||||
|
|
||||||
|
python pty_forward.py -- container exec --interactive --tty <name> <cmd>
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
|
import subprocess
|
||||||
|
import sys
|
||||||
|
import termios
|
||||||
|
import tty
|
||||||
|
|
||||||
|
|
||||||
|
def main(argv: list[str]) -> int:
|
||||||
|
"""Entry point. ``argv`` shape: ``-- <inner-argv...>``."""
|
||||||
|
if len(argv) < 2 or argv[0] != "--":
|
||||||
|
sys.stderr.write(
|
||||||
|
"usage: python pty_forward.py -- <container-exec-argv...>\n"
|
||||||
|
)
|
||||||
|
return 2
|
||||||
|
inner = argv[1:]
|
||||||
|
|
||||||
|
try:
|
||||||
|
fd = sys.stdin.fileno()
|
||||||
|
except OSError:
|
||||||
|
return subprocess.run(inner, check=False).returncode
|
||||||
|
|
||||||
|
if not os.isatty(fd):
|
||||||
|
return subprocess.run(inner, check=False).returncode
|
||||||
|
|
||||||
|
try:
|
||||||
|
old = termios.tcgetattr(fd)
|
||||||
|
except termios.error:
|
||||||
|
return subprocess.run(inner, check=False).returncode
|
||||||
|
|
||||||
|
try:
|
||||||
|
tty.setraw(fd)
|
||||||
|
return subprocess.run(inner, check=False).returncode
|
||||||
|
finally:
|
||||||
|
termios.tcsetattr(fd, termios.TCSADRAIN, old)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
sys.exit(main(sys.argv[1:]))
|
||||||
@@ -2,15 +2,16 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import sys
|
||||||
import unittest
|
import unittest
|
||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
from bot_bottle.backend.macos_container import bottle as bottle_mod
|
from bot_bottle.backend.macos_container import bottle as bottle_mod
|
||||||
from bot_bottle.backend.macos_container.bottle import MacosContainerBottle
|
from bot_bottle.backend.macos_container.bottle import MacosContainerBottle, _PTY_FORWARD_SCRIPT
|
||||||
|
|
||||||
|
|
||||||
class TestMacosContainerBottle(unittest.TestCase):
|
class TestMacosContainerBottle(unittest.TestCase):
|
||||||
def test_agent_argv_uses_container_exec(self):
|
def test_agent_argv_uses_pty_forward_and_container_exec(self):
|
||||||
bottle = MacosContainerBottle(
|
bottle = MacosContainerBottle(
|
||||||
"bot-bottle-dev-abc",
|
"bot-bottle-dev-abc",
|
||||||
lambda: None,
|
lambda: None,
|
||||||
@@ -21,6 +22,7 @@ class TestMacosContainerBottle(unittest.TestCase):
|
|||||||
argv = bottle.agent_argv(["run"])
|
argv = bottle.agent_argv(["run"])
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
[
|
[
|
||||||
|
sys.executable, _PTY_FORWARD_SCRIPT, "--",
|
||||||
"container", "exec", "--interactive", "--tty",
|
"container", "exec", "--interactive", "--tty",
|
||||||
"--env", "TERM=xterm-256color",
|
"--env", "TERM=xterm-256color",
|
||||||
"bot-bottle-dev-abc", "codex", "run",
|
"bot-bottle-dev-abc", "codex", "run",
|
||||||
@@ -39,6 +41,7 @@ class TestMacosContainerBottle(unittest.TestCase):
|
|||||||
argv = bottle.agent_argv([])
|
argv = bottle.agent_argv([])
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
[
|
[
|
||||||
|
sys.executable, _PTY_FORWARD_SCRIPT, "--",
|
||||||
"container", "exec", "--interactive", "--tty",
|
"container", "exec", "--interactive", "--tty",
|
||||||
"--env", "TERM=xterm-256color",
|
"--env", "TERM=xterm-256color",
|
||||||
"--workdir", "/home/node/workspace",
|
"--workdir", "/home/node/workspace",
|
||||||
@@ -51,7 +54,6 @@ class TestMacosContainerBottle(unittest.TestCase):
|
|||||||
bottle = MacosContainerBottle("bot-bottle-dev-abc", lambda: None, None)
|
bottle = MacosContainerBottle("bot-bottle-dev-abc", lambda: None, None)
|
||||||
with patch.dict(bottle_mod.os.environ, {"TERM": "screen-256color"}, clear=False):
|
with patch.dict(bottle_mod.os.environ, {"TERM": "screen-256color"}, clear=False):
|
||||||
argv = bottle.agent_argv([])
|
argv = bottle.agent_argv([])
|
||||||
self.assertIn("--env", argv)
|
|
||||||
self.assertIn("TERM=screen-256color", argv)
|
self.assertIn("TERM=screen-256color", argv)
|
||||||
|
|
||||||
def test_agent_argv_term_falls_back_to_xterm_256color(self):
|
def test_agent_argv_term_falls_back_to_xterm_256color(self):
|
||||||
@@ -61,11 +63,13 @@ class TestMacosContainerBottle(unittest.TestCase):
|
|||||||
argv = bottle.agent_argv([])
|
argv = bottle.agent_argv([])
|
||||||
self.assertIn("TERM=xterm-256color", argv)
|
self.assertIn("TERM=xterm-256color", argv)
|
||||||
|
|
||||||
def test_agent_argv_no_tty_omits_term(self):
|
def test_agent_argv_no_tty_omits_wrapper_and_tty_flags(self):
|
||||||
bottle = MacosContainerBottle("bot-bottle-dev-abc", lambda: None, None)
|
bottle = MacosContainerBottle("bot-bottle-dev-abc", lambda: None, None)
|
||||||
argv = bottle.agent_argv([], tty=False)
|
argv = bottle.agent_argv([], tty=False)
|
||||||
self.assertNotIn("--tty", argv)
|
self.assertNotIn("--tty", argv)
|
||||||
self.assertNotIn("--env", argv)
|
self.assertNotIn("--env", argv)
|
||||||
|
self.assertNotIn(_PTY_FORWARD_SCRIPT, argv)
|
||||||
|
self.assertEqual(["container", "exec", "bot-bottle-dev-abc", "claude"], argv)
|
||||||
|
|
||||||
def test_exec_pipes_script_to_shell(self):
|
def test_exec_pipes_script_to_shell(self):
|
||||||
bottle = MacosContainerBottle("bot-bottle-dev-abc", lambda: None, None)
|
bottle = MacosContainerBottle("bot-bottle-dev-abc", lambda: None, None)
|
||||||
|
|||||||
@@ -0,0 +1,136 @@
|
|||||||
|
"""Unit: macos-container pty_forward raw-mode wrapper (issue #245).
|
||||||
|
|
||||||
|
Tests argument parsing, non-TTY fallback, and the raw-mode
|
||||||
|
setup/restore sequence without requiring a real terminal.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import io
|
||||||
|
import termios
|
||||||
|
import unittest
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
from bot_bottle.backend.macos_container import pty_forward
|
||||||
|
|
||||||
|
|
||||||
|
def _fake_stdin(fd: int = 0) -> MagicMock:
|
||||||
|
"""Return a mock stdin whose fileno() returns *fd*."""
|
||||||
|
m = MagicMock()
|
||||||
|
m.fileno.return_value = fd
|
||||||
|
return m
|
||||||
|
|
||||||
|
|
||||||
|
class TestArgvParsing(unittest.TestCase):
|
||||||
|
def test_missing_separator_returns_error_exit_code(self):
|
||||||
|
with patch.object(pty_forward.sys, "stderr", new=io.StringIO()) as err:
|
||||||
|
rc = pty_forward.main(["container", "exec"])
|
||||||
|
self.assertEqual(2, rc)
|
||||||
|
self.assertIn("usage:", err.getvalue())
|
||||||
|
|
||||||
|
def test_too_few_args_returns_error_exit_code(self):
|
||||||
|
with patch.object(pty_forward.sys, "stderr", new=io.StringIO()):
|
||||||
|
self.assertEqual(2, pty_forward.main([]))
|
||||||
|
self.assertEqual(2, pty_forward.main(["--"]))
|
||||||
|
|
||||||
|
def test_separator_at_start_with_inner_is_valid(self):
|
||||||
|
with (
|
||||||
|
patch.object(pty_forward.sys, "stdin", _fake_stdin()),
|
||||||
|
patch.object(pty_forward.os, "isatty", return_value=False),
|
||||||
|
patch.object(pty_forward.subprocess, "run") as run,
|
||||||
|
):
|
||||||
|
run.return_value.returncode = 0
|
||||||
|
rc = pty_forward.main(["--", "container", "exec"])
|
||||||
|
self.assertEqual(0, rc)
|
||||||
|
run.assert_called_once_with(["container", "exec"], check=False)
|
||||||
|
|
||||||
|
|
||||||
|
class TestNonTtyFallback(unittest.TestCase):
|
||||||
|
def test_non_tty_stdin_runs_inner_directly(self):
|
||||||
|
with (
|
||||||
|
patch.object(pty_forward.sys, "stdin", _fake_stdin()),
|
||||||
|
patch.object(pty_forward.os, "isatty", return_value=False),
|
||||||
|
patch.object(pty_forward.subprocess, "run") as run,
|
||||||
|
):
|
||||||
|
run.return_value.returncode = 42
|
||||||
|
rc = pty_forward.main(
|
||||||
|
["--", "container", "exec", "--interactive", "--tty", "c", "claude"]
|
||||||
|
)
|
||||||
|
self.assertEqual(42, rc)
|
||||||
|
run.assert_called_once_with(
|
||||||
|
["container", "exec", "--interactive", "--tty", "c", "claude"],
|
||||||
|
check=False,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_fileno_error_runs_inner_directly(self):
|
||||||
|
bad_stdin = MagicMock()
|
||||||
|
bad_stdin.fileno.side_effect = OSError("pseudofile")
|
||||||
|
with (
|
||||||
|
patch.object(pty_forward.sys, "stdin", bad_stdin),
|
||||||
|
patch.object(pty_forward.subprocess, "run") as run,
|
||||||
|
):
|
||||||
|
run.return_value.returncode = 0
|
||||||
|
rc = pty_forward.main(["--", "container", "exec"])
|
||||||
|
run.assert_called_once_with(["container", "exec"], check=False)
|
||||||
|
self.assertEqual(0, rc)
|
||||||
|
|
||||||
|
|
||||||
|
class TestRawModeSetupAndRestore(unittest.TestCase):
|
||||||
|
def test_tty_stdin_sets_raw_mode_and_restores_on_exit(self):
|
||||||
|
saved_attrs = object()
|
||||||
|
with (
|
||||||
|
patch.object(pty_forward.sys, "stdin", _fake_stdin()),
|
||||||
|
patch.object(pty_forward.os, "isatty", return_value=True),
|
||||||
|
patch.object(pty_forward.termios, "tcgetattr", return_value=saved_attrs),
|
||||||
|
patch.object(pty_forward.tty, "setraw") as setraw,
|
||||||
|
patch.object(pty_forward.termios, "tcsetattr") as tcsetattr,
|
||||||
|
patch.object(pty_forward.subprocess, "run") as run,
|
||||||
|
):
|
||||||
|
run.return_value.returncode = 0
|
||||||
|
rc = pty_forward.main(["--", "container", "exec"])
|
||||||
|
|
||||||
|
self.assertEqual(0, rc)
|
||||||
|
setraw.assert_called_once()
|
||||||
|
tcsetattr.assert_called_once_with(
|
||||||
|
unittest.mock.ANY, termios.TCSADRAIN, saved_attrs,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_tty_restores_on_subprocess_nonzero_exit(self):
|
||||||
|
saved_attrs = object()
|
||||||
|
with (
|
||||||
|
patch.object(pty_forward.sys, "stdin", _fake_stdin()),
|
||||||
|
patch.object(pty_forward.os, "isatty", return_value=True),
|
||||||
|
patch.object(pty_forward.termios, "tcgetattr", return_value=saved_attrs),
|
||||||
|
patch.object(pty_forward.tty, "setraw"),
|
||||||
|
patch.object(pty_forward.termios, "tcsetattr") as tcsetattr,
|
||||||
|
patch.object(pty_forward.subprocess, "run") as run,
|
||||||
|
):
|
||||||
|
run.return_value.returncode = 1
|
||||||
|
rc = pty_forward.main(["--", "container", "exec"])
|
||||||
|
|
||||||
|
self.assertEqual(1, rc)
|
||||||
|
tcsetattr.assert_called_once_with(
|
||||||
|
unittest.mock.ANY, termios.TCSADRAIN, saved_attrs,
|
||||||
|
)
|
||||||
|
|
||||||
|
def test_tcgetattr_error_falls_back_to_bare_run(self):
|
||||||
|
with (
|
||||||
|
patch.object(pty_forward.sys, "stdin", _fake_stdin()),
|
||||||
|
patch.object(pty_forward.os, "isatty", return_value=True),
|
||||||
|
patch.object(
|
||||||
|
pty_forward.termios, "tcgetattr",
|
||||||
|
side_effect=termios.error("not a tty"),
|
||||||
|
),
|
||||||
|
patch.object(pty_forward.tty, "setraw") as setraw,
|
||||||
|
patch.object(pty_forward.subprocess, "run") as run,
|
||||||
|
):
|
||||||
|
run.return_value.returncode = 0
|
||||||
|
rc = pty_forward.main(["--", "container", "exec"])
|
||||||
|
|
||||||
|
setraw.assert_not_called()
|
||||||
|
run.assert_called_once_with(["container", "exec"], check=False)
|
||||||
|
self.assertEqual(0, rc)
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
unittest.main()
|
||||||
Reference in New Issue
Block a user