fix(web): attach terminals through configured backend (#417)

* fix(web): attach terminals through configured backend

Route web terminal attachment through the configured terminal backend, preserve tmux behavior, focus the requested Herdr tab before launching its TUI, and handle backend attachment failures safely.

Squash-rebased onto awslabs/cli-agent-orchestrator main at 67f8e4b.

* chore: add one-use PR #417 changelog workflow

* docs: add PR #417 changelog entry

* chore: add one-use PR #417 validation workflow

* chore: complete PR #417 validation

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: haofeif <56006724+haofeif@users.noreply.github.com>

Author: 3 people

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- web: attach web terminals through the configured backend so herdr-backed terminals no longer fail to attach (#417)
 - honor profile frontmatter `provider:` during install (flag > frontmatter > default) (#414)
 ### Security
 

src/cli_agent_orchestrator/api/main.py

@@ -35,7 +35,7 @@
 from fastapi.responses import JSONResponse
 from pydantic import BaseModel, Field, field_validator, model_validator
 
-from cli_agent_orchestrator.backends import TerminalNotFoundError
+from cli_agent_orchestrator.backends import TerminalBackendError, TerminalNotFoundError
 from cli_agent_orchestrator.backends.herdr_backend import HerdrBackend
 from cli_agent_orchestrator.backends.registry import get_backend
 from cli_agent_orchestrator.clients.database import (
@@ -2050,29 +2050,39 @@ async def terminal_ws(websocket: WebSocket, terminal_id: str):
     # or future code paths could still bypass that, and tmux parses
     # ':' / '.' as target delimiters. Bind the validator return values
     # so the sanitization is explicit at the actual sink below.
+    # This tmux-shaped validation is deliberately applied to every backend.
     try:
         session_name = validate_tmux_name(metadata["tmux_session"], "session_name")
         window_name = validate_tmux_name(metadata["tmux_window"], "window_name")
     except ValueError:
         await websocket.close(code=4003, reason="Invalid tmux target name")
         return
 
-    # Create PTY pair for tmux attach
+    try:
+        attach_command = await asyncio.to_thread(
+            get_backend().prepare_web_attach, session_name, window_name
+        )
+    except TerminalBackendError as e:
+        logger.error(f"Web attach failed for terminal {terminal_id}: {e}")
+        await websocket.close(code=4004, reason="Failed to attach terminal")
+        return
+
+    # Create PTY pair for backend attach
     master_fd, slave_fd = pty.openpty()
 
     # Set initial terminal size
     winsize = struct.pack("HHHH", 24, 80, 0, 0)
     fcntl.ioctl(slave_fd, termios.TIOCSWINSZ, winsize)
 
-    # Start tmux attach inside the PTY.
+    # Start the configured backend's interactive client inside the PTY.
     # Container/devcontainer environments often leave TERM unset or set to
     # ``dumb``, which strips colours, breaks cursor positioning and corrupts
     # the Ink-based TUIs that agent CLIs render. Force a sane default so the
     # browser-side xterm.js renderer sees the escape sequences it expects.
     # Any explicit non-dumb TERM the operator set is preserved.
     pty_env = _build_pty_env()
     proc = subprocess.Popen(
-        ["tmux", "-u", "attach-session", "-t", f"{session_name}:{window_name}"],
+        attach_command,
         stdin=slave_fd,
         stdout=slave_fd,
         stderr=slave_fd,

src/cli_agent_orchestrator/backends/base.py

@@ -238,6 +238,25 @@ def attach_session(self, session_name: str) -> None:
         """
         ...
 
+    @abstractmethod
+    def prepare_web_attach(self, session_name: str, window_name: str) -> List[str]:
+        """Prepare a browser PTY attachment and return its subprocess argv.
+
+        Backends may perform routing work before returning, such as focusing a
+        Herdr workspace/tab. The caller owns the PTY and subprocess lifecycle.
+
+        Args:
+            session_name: Target session
+            window_name: Target window
+
+        Returns:
+            Subprocess argv for the interactive backend client
+
+        Raises:
+            TerminalBackendError: If the backend cannot prepare the attachment
+        """
+        ...
+
     # --- Pipe-pane (logging) ---
 
     @abstractmethod

src/cli_agent_orchestrator/backends/herdr_backend.py

@@ -543,6 +543,13 @@ def attach_session(self, session_name: str) -> None:
         # Equivalent to `tmux attach-session -t <session>`.
         os.execvp("herdr", ["herdr", "--session", self._herdr_session])
 
+    def prepare_web_attach(self, session_name: str, window_name: str) -> List[str]:
+        """Focus the requested Herdr tab and return the browser PTY attach command."""
+        workspace_id = self._resolve_workspace_id(session_name)
+        tab_id = self._resolve_tab_id(session_name, workspace_id, window_name)
+        self._run_herdr(["tab", "focus", tab_id])
+        return ["herdr", "--session", self._herdr_session]
+
     # --- Capability overrides ---
 
     def supports_event_inbox(self) -> bool:

src/cli_agent_orchestrator/backends/tmux_backend.py

@@ -134,6 +134,10 @@ def attach_session(self, session_name: str) -> None:
 
         subprocess.run(["tmux", "attach-session", "-t", session_name], check=True)
 
+    def prepare_web_attach(self, session_name: str, window_name: str) -> List[str]:
+        """Return the tmux command used by the browser PTY WebSocket."""
+        return ["tmux", "-u", "attach-session", "-t", f"{session_name}:{window_name}"]
+
     # --- Pipe-pane ---
 
     def pipe_pane(self, session_name: str, window_name: str, file_path: str) -> None:

test/api/test_terminals.py

@@ -613,13 +613,23 @@ def capture_and_stop(*args, **kwargs):
             captured["kwargs"] = kwargs
             raise _StopHere("captured Popen args")
 
+        backend = MagicMock()
+        backend.prepare_web_attach.return_value = [
+            "tmux",
+            "-u",
+            "attach-session",
+            "-t",
+            "cao-s:w",
+        ]
+
         with (
             patch.dict("os.environ", {"TERM": "dumb", "HOME": "/root"}, clear=True),
             patch.object(
                 main_module,
                 "get_terminal_metadata",
                 return_value={"tmux_session": "cao-s", "tmux_window": "w"},
             ),
+            patch.object(main_module, "get_backend", return_value=backend),
             patch.object(main_module.subprocess, "Popen", side_effect=capture_and_stop),
             patch.object(main_module.pty, "openpty", return_value=(100, 101)),
             patch.object(main_module.fcntl, "ioctl"),
@@ -636,6 +646,72 @@ def capture_and_stop(*args, **kwargs):
         )
         assert passed_env["TERM"] == "xterm-256color"
 
+    @pytest.mark.asyncio
+    async def test_websocket_uses_configured_backend_attach_command(self):
+        """Herdr-backed terminals must not be attached through tmux."""
+        from cli_agent_orchestrator.api import main as main_module
+
+        ws = MagicMock()
+        ws.client = MagicMock(host="127.0.0.1")
+        ws.accept = AsyncMock()
+        ws.close = AsyncMock()
+
+        backend = MagicMock()
+        backend.prepare_web_attach.return_value = ["herdr", "--session", "cao"]
+
+        captured: Dict[str, object] = {}
+
+        def capture_and_stop(*args, **kwargs):
+            captured["args"] = args
+            raise _StopHere("captured Popen args")
+
+        with (
+            patch.object(
+                main_module,
+                "get_terminal_metadata",
+                return_value={"tmux_session": "cao-s", "tmux_window": "w"},
+            ),
+            patch.object(main_module, "get_backend", return_value=backend),
+            patch.object(main_module.subprocess, "Popen", side_effect=capture_and_stop),
+            patch.object(main_module.pty, "openpty", return_value=(100, 101)),
+            patch.object(main_module.fcntl, "ioctl"),
+            patch.object(main_module.fcntl, "fcntl"),
+            patch.object(main_module.os, "close"),
+        ):
+            with pytest.raises(_StopHere):
+                await main_module.terminal_ws(ws, "abcd1234")
+
+        backend.prepare_web_attach.assert_called_once_with("cao-s", "w")
+        assert captured["args"][0] == ["herdr", "--session", "cao"]
+
+    @pytest.mark.asyncio
+    async def test_websocket_closes_safely_when_backend_attach_fails(self):
+        """Backend attach errors close safely before allocating a PTY."""
+        from cli_agent_orchestrator.api import main as main_module
+        from cli_agent_orchestrator.backends import TerminalBackendError
+
+        ws = MagicMock()
+        ws.client = MagicMock(host="127.0.0.1")
+        ws.accept = AsyncMock()
+        ws.close = AsyncMock()
+
+        backend = MagicMock()
+        backend.prepare_web_attach.side_effect = TerminalBackendError("sensitive backend detail")
+
+        with (
+            patch.object(
+                main_module,
+                "get_terminal_metadata",
+                return_value={"tmux_session": "cao-s", "tmux_window": "w"},
+            ),
+            patch.object(main_module, "get_backend", return_value=backend),
+            patch.object(main_module.pty, "openpty") as mock_openpty,
+        ):
+            await main_module.terminal_ws(ws, "abcd1234")
+
+        ws.close.assert_awaited_once_with(code=4004, reason="Failed to attach terminal")
+        mock_openpty.assert_not_called()
+
 
 class _StopHere(Exception):
     """Sentinel raised by the wiring test once Popen args are captured."""

test/backends/test_herdr_backend.py

@@ -115,6 +115,35 @@ def test_list_sessions(self, mock_run, backend):
         assert result[0]["name"] == "cao-proj1"
         assert result[1]["name"] == "cao-proj2"
 
+    @patch("subprocess.run")
+    def test_prepare_web_attach_focuses_tab_and_returns_herdr_command(self, mock_run, backend):
+        """Browser attachment focuses the requested Herdr tab before opening its TUI."""
+        ws = [{"label": "cao-test", "workspace_id": "w1"}]
+        tabs = [{"tab_id": "tab-1", "workspace_id": "w1", "label": "developer-abcd"}]
+        mock_run.side_effect = [
+            _completed(_make_workspace_list_response(ws)),
+            _completed(_make_tab_list_response(tabs)),
+            _completed(),
+        ]
+
+        command = backend.prepare_web_attach("cao-test", "developer-abcd")
+
+        assert command == ["herdr", "--session", "cao"]
+        assert mock_run.call_args_list[-1].args[0][-3:] == ["tab", "focus", "tab-1"]
+
+    def test_prepare_web_attach_propagates_tab_not_found(self, backend):
+        """Browser attachment propagates a missing requested Herdr tab."""
+        error = TerminalNotFoundError("cao-test:missing-window")
+
+        with (
+            patch.object(backend, "_resolve_workspace_id", return_value="w1"),
+            patch.object(backend, "_resolve_tab_id", side_effect=error),
+            pytest.raises(TerminalNotFoundError) as exc_info,
+        ):
+            backend.prepare_web_attach("cao-test", "missing-window")
+
+        assert exc_info.value is error
+
     @patch("subprocess.run")
     def test_create_session_calls_workspace_create(self, mock_run, backend):
         """create_session should call herdr workspace create with --label and inject env."""

test/backends/test_tmux_backend.py

@@ -74,6 +74,15 @@ def test_list_sessions_delegates(self, backend, mock_client):
         assert len(result) == 1
         mock_client.list_sessions.assert_called_once()
 
+    def test_prepare_web_attach_returns_window_target(self, backend):
+        assert backend.prepare_web_attach("cao-test", "developer-abcd") == [
+            "tmux",
+            "-u",
+            "attach-session",
+            "-t",
+            "cao-test:developer-abcd",
+        ]
+
     def test_kill_session_delegates(self, backend, mock_client):
         mock_client.kill_session.return_value = True
         assert backend.kill_session("cao-test") is True