From db1f2740690daa7e0e529cf4172dbe9b3110b298 Mon Sep 17 00:00:00 2001 From: sladro Date: Mon, 6 Apr 2026 10:37:47 +0800 Subject: [PATCH] fix: harden mobile terminal input and reconnect --- .../lib/features/terminal/terminal_page.dart | 14 ++- .../terminal_session_coordinator.dart | 116 ++++++++++++++++-- .../terminal/terminal_page_input_test.dart | 25 ++++ .../terminal_session_coordinator_test.dart | 98 +++++++++++++++ 4 files changed, 238 insertions(+), 15 deletions(-) diff --git a/apps/mobile_app/lib/features/terminal/terminal_page.dart b/apps/mobile_app/lib/features/terminal/terminal_page.dart index f2b2570..8ff1887 100644 --- a/apps/mobile_app/lib/features/terminal/terminal_page.dart +++ b/apps/mobile_app/lib/features/terminal/terminal_page.dart @@ -130,8 +130,9 @@ class _TerminalPageState extends ConsumerState _coordinator.handleTerminalResize(width, height); }; terminal.onOutput = (data) { - _diagnosticLog.add('ui.terminal.key', data); - _coordinator.sendInput(data); + final normalizedInput = _normalizeTerminalKeyboardInput(data); + _diagnosticLog.add('ui.terminal.key', normalizedInput); + _coordinator.sendInput(normalizedInput); }; unawaited(_coordinator.start()); } @@ -387,6 +388,14 @@ class _TerminalPageState extends ConsumerState return text.replaceAll('\r\n', '\n').replaceAll('\r', '\n'); } + static String _normalizeTerminalKeyboardInput(String input) { + if (!input.contains('\n')) { + return input; + } + + return input.replaceAll('\r\n', '\r').replaceAll('\n', '\r'); + } + Future _showDiagnostics() { return showModalBottomSheet( context: context, @@ -707,6 +716,7 @@ class _TerminalPageState extends ConsumerState focusNode: _terminalFocusNode, autofocus: false, keyboardType: TextInputType.multiline, + deleteDetection: true, scrollController: _terminalScrollController, ), ), diff --git a/apps/mobile_app/lib/features/terminal/terminal_session_coordinator.dart b/apps/mobile_app/lib/features/terminal/terminal_session_coordinator.dart index bac6bb2..5b70adc 100644 --- a/apps/mobile_app/lib/features/terminal/terminal_session_coordinator.dart +++ b/apps/mobile_app/lib/features/terminal/terminal_session_coordinator.dart @@ -12,6 +12,9 @@ import 'terminal_socket_session.dart'; typedef CancelReconnect = void Function(); typedef ReconnectScheduler = CancelReconnect Function(Duration delay, Future Function() callback); +typedef CancelResize = void Function(); +typedef ResizeScheduler = + CancelResize Function(Duration delay, void Function() callback); typedef TerminalSessionFactory = TerminalSocketSession Function({ required Uri baseUri, @@ -37,8 +40,10 @@ class TerminalSessionCoordinator extends ChangeNotifier { this.onHistoryLoaded, this.diagnosticLog, ReconnectScheduler? reconnectScheduler, + ResizeScheduler? resizeScheduler, }) : baseUri = baseUri ?? _defaultBaseUri, - _reconnectScheduler = reconnectScheduler ?? _defaultReconnectScheduler; + _reconnectScheduler = reconnectScheduler ?? _defaultReconnectScheduler, + _resizeScheduler = resizeScheduler ?? _defaultResizeScheduler; static final Uri _defaultBaseUri = Uri( scheme: 'https', @@ -46,6 +51,7 @@ class TerminalSessionCoordinator extends ChangeNotifier { port: 9443, ); static const Duration reconnectDelay = Duration(seconds: 1); + static const Duration resizeDebounceDelay = Duration(milliseconds: 120); static const int initialHistoryLineCount = 200; static const int pendingInputCharacterLimit = 2048; @@ -59,9 +65,11 @@ class TerminalSessionCoordinator extends ChangeNotifier { final void Function(HistoryWindow history)? onHistoryLoaded; final TerminalDiagnosticLog? diagnosticLog; final ReconnectScheduler _reconnectScheduler; + final ResizeScheduler _resizeScheduler; TerminalSocketSession? _socketSession; CancelReconnect? _cancelReconnect; + CancelResize? _cancelResize; int _historyLineCount = initialHistoryLineCount; bool _isLoadingOlderHistory = false; bool _isDisposed = false; @@ -70,6 +78,11 @@ class TerminalSessionCoordinator extends ChangeNotifier { String _connectionStatus = 'Connecting...'; final List _pendingInputs = []; int _pendingInputCharacterCount = 0; + int _sessionGeneration = 0; + int? _pendingResizeColumns; + int? _pendingResizeRows; + int? _lastSentColumns; + int? _lastSentRows; bool get isLoadingOlderHistory => _isLoadingOlderHistory; @@ -77,12 +90,15 @@ class TerminalSessionCoordinator extends ChangeNotifier { Future start({bool isReconnect = false}) async { _cancelPendingReconnect(); + _cancelPendingResize(); _reconnectPending = false; if (_isDisposed) { return; } + final sessionGeneration = ++_sessionGeneration; + if (isReconnect) { controller.markReconnecting(); _connectionStatus = 'Reconnecting to ${session.name}...'; @@ -110,9 +126,15 @@ class TerminalSessionCoordinator extends ChangeNotifier { try { await socketSession.connect( - onFrame: _handleFrame, + onFrame: (chunk) { + if (!_isCurrentSession(socketSession, sessionGeneration)) { + return; + } + + _handleFrame(chunk); + }, onDisconnected: () { - if (_isDisposed || !identical(_socketSession, socketSession)) { + if (!_isCurrentSession(socketSession, sessionGeneration)) { return; } @@ -120,23 +142,19 @@ class TerminalSessionCoordinator extends ChangeNotifier { }, ); - if (_isDisposed || !identical(_socketSession, socketSession)) { + if (!_isCurrentSession(socketSession, sessionGeneration)) { return; } final viewport = viewportProvider(); - socketSession.sendResize(viewport.columns, viewport.rows); - diagnosticLog?.add( - 'socket.resize.send', - '${viewport.columns}x${viewport.rows}', - ); + _sendResize(socketSession, viewport.columns, viewport.rows); _flushPendingInputs(socketSession); controller.markConnected(); _connectionStatus = 'Attached to ${session.name}'; diagnosticLog?.add('socket.attach.ack', session.sessionId); notifyListeners(); } catch (error) { - if (_isDisposed || !identical(_socketSession, socketSession)) { + if (!_isCurrentSession(socketSession, sessionGeneration)) { return; } @@ -146,13 +164,22 @@ class TerminalSessionCoordinator extends ChangeNotifier { notifyListeners(); _scheduleReconnect(); } finally { - _connectionAttemptInProgress = false; + if (sessionGeneration == _sessionGeneration) { + _connectionAttemptInProgress = false; + } } } void handleTerminalResize(int columns, int rows) { diagnosticLog?.add('ui.terminal.resize', '${columns}x${rows}'); - _socketSession?.sendResize(columns, rows); + if (columns <= 0 || rows <= 0 || _socketSession == null) { + return; + } + + _pendingResizeColumns = columns; + _pendingResizeRows = rows; + _cancelResize?.call(); + _cancelResize = _resizeScheduler(resizeDebounceDelay, _flushPendingResize); } void sendInput(String input) { @@ -224,6 +251,7 @@ class TerminalSessionCoordinator extends ChangeNotifier { Future suspendForBackground() async { _cancelPendingReconnect(); + _cancelPendingResize(); if (_isDisposed) { return; @@ -239,6 +267,7 @@ class TerminalSessionCoordinator extends ChangeNotifier { Future close() async { _isDisposed = true; _cancelPendingReconnect(); + _cancelPendingResize(); _pendingInputs.clear(); _pendingInputCharacterCount = 0; await _closeActiveSession(); @@ -274,6 +303,7 @@ class TerminalSessionCoordinator extends ChangeNotifier { void _scheduleReconnect() { _cancelPendingReconnect(); + _cancelPendingResize(); _reconnectPending = true; controller.markReconnecting(); _connectionStatus = 'Connection lost. Reconnecting...'; @@ -295,6 +325,7 @@ class TerminalSessionCoordinator extends ChangeNotifier { } void _disposeActiveSessionInBackground() { + _cancelPendingResize(); final activeSession = _socketSession; _socketSession = null; if (activeSession != null) { @@ -303,6 +334,7 @@ class TerminalSessionCoordinator extends ChangeNotifier { } Future _closeActiveSession() async { + _cancelPendingResize(); final activeSession = _socketSession; _socketSession = null; if (activeSession != null) { @@ -320,6 +352,14 @@ class TerminalSessionCoordinator extends ChangeNotifier { return timer.cancel; } + static CancelResize _defaultResizeScheduler( + Duration delay, + void Function() callback, + ) { + final timer = Timer(delay, callback); + return timer.cancel; + } + static String _formatInputForDiagnostics(String input) { return input.replaceAll('\r', r'\r').replaceAll('\n', r'\n'); } @@ -359,7 +399,10 @@ class TerminalSessionCoordinator extends ChangeNotifier { final input = pendingInputs[index]; final result = socketSession.sendInput(input); if (result == TerminalSocketDispatchResult.sent) { - diagnosticLog?.add('socket.input.flush', _formatInputForDiagnostics(input)); + diagnosticLog?.add( + 'socket.input.flush', + _formatInputForDiagnostics(input), + ); continue; } @@ -372,4 +415,51 @@ class TerminalSessionCoordinator extends ChangeNotifier { break; } } + + void _flushPendingResize() { + _cancelResize = null; + + if (_isDisposed) { + return; + } + + final socketSession = _socketSession; + final columns = _pendingResizeColumns; + final rows = _pendingResizeRows; + _pendingResizeColumns = null; + _pendingResizeRows = null; + + if (socketSession == null || columns == null || rows == null) { + return; + } + + if (_lastSentColumns == columns && _lastSentRows == rows) { + return; + } + + _sendResize(socketSession, columns, rows); + } + + void _cancelPendingResize() { + _cancelResize?.call(); + _cancelResize = null; + _pendingResizeColumns = null; + _pendingResizeRows = null; + } + + void _sendResize(TerminalSocketSession socketSession, int columns, int rows) { + socketSession.sendResize(columns, rows); + _lastSentColumns = columns; + _lastSentRows = rows; + diagnosticLog?.add('socket.resize.send', '${columns}x${rows}'); + } + + bool _isCurrentSession( + TerminalSocketSession socketSession, + int sessionGeneration, + ) { + return !_isDisposed && + sessionGeneration == _sessionGeneration && + identical(_socketSession, socketSession); + } } diff --git a/apps/mobile_app/test/features/terminal/terminal_page_input_test.dart b/apps/mobile_app/test/features/terminal/terminal_page_input_test.dart index 38088c4..28ebd51 100644 --- a/apps/mobile_app/test/features/terminal/terminal_page_input_test.dart +++ b/apps/mobile_app/test/features/terminal/terminal_page_input_test.dart @@ -46,6 +46,31 @@ void main() { expect(terminalView.keyboardType, TextInputType.multiline); }); + testWidgets('terminal view enables mobile delete detection', (tester) async { + await _pumpTerminalPage(tester); + + final terminalView = tester.widget(find.byType(TerminalView)); + + expect(terminalView.deleteDetection, isTrue); + }); + + testWidgets('soft keyboard newline is sent as terminal enter', ( + tester, + ) async { + final transportFactory = _QueuedTerminalSocketTransportFactory(); + + await _pumpTerminalPage(tester, socketFactory: transportFactory.factory); + + final terminalView = tester.widget(find.byType(TerminalView)); + terminalView.terminal.onOutput?.call('\n'); + await tester.pumpAndSettle(); + + expect( + transportFactory.createdTransports.single.sentMessages.last, + contains(r'"input":"\r"'), + ); + }); + testWidgets('terminal actions sheet unifies session actions and quick keys', ( tester, ) async { diff --git a/apps/mobile_app/test/features/terminal/terminal_session_coordinator_test.dart b/apps/mobile_app/test/features/terminal/terminal_session_coordinator_test.dart index 388b4fa..d787c94 100644 --- a/apps/mobile_app/test/features/terminal/terminal_session_coordinator_test.dart +++ b/apps/mobile_app/test/features/terminal/terminal_session_coordinator_test.dart @@ -232,6 +232,85 @@ void main() { expect(controller.liveLines.last, 'line-299'); expect(controller.liveLines, isNot(contains('line-0'))); }); + + test( + 'rapid resize updates are coalesced into the final stable viewport', + () async { + final controller = TerminalInteractionController(); + final apiClient = _FakeAgentApiClient(); + final sessionFactory = _FakeTerminalSessionFactory(); + final resizeScheduler = _FakeResizeScheduler(); + final session = Session( + sessionId: 'abc', + name: 'codex-main', + status: 'idle', + ); + final coordinator = TerminalSessionCoordinator( + controller: controller, + apiClient: apiClient, + session: session, + sessionFactory: sessionFactory.create, + onFrame: (_) {}, + viewportProvider: () => const TerminalViewport(columns: 80, rows: 24), + resizeScheduler: resizeScheduler.schedule, + ); + + await coordinator.start(); + final socketSession = sessionFactory.createdSessions.single; + + coordinator.handleTerminalResize(100, 30); + coordinator.handleTerminalResize(98, 26); + + expect(socketSession.resizeCalls, const [ + [80, 24], + ]); + expect(resizeScheduler.pendingCallback, isNotNull); + + resizeScheduler.runPending(); + + expect(socketSession.resizeCalls, const [ + [80, 24], + [98, 26], + ]); + }, + ); + + test( + 'frames from a replaced socket session are ignored after reconnect starts', + () async { + final controller = TerminalInteractionController(); + final apiClient = _FakeAgentApiClient(); + final sessionFactory = _FakeTerminalSessionFactory(); + final session = Session( + sessionId: 'abc', + name: 'codex-main', + status: 'idle', + ); + final receivedFrames = []; + final coordinator = TerminalSessionCoordinator( + controller: controller, + apiClient: apiClient, + session: session, + sessionFactory: sessionFactory.create, + onFrame: receivedFrames.add, + viewportProvider: () => const TerminalViewport(columns: 80, rows: 24), + ); + + await coordinator.start(); + final firstSession = sessionFactory.createdSessions.first; + firstSession.disposeCompleter = Completer(); + + await coordinator.reconnectNow(); + + expect(sessionFactory.createdSessions, hasLength(2)); + final secondSession = sessionFactory.createdSessions.last; + + firstSession.emitFrame('stale-frame'); + secondSession.emitFrame('fresh-frame'); + + expect(receivedFrames, ['fresh-frame']); + }, + ); } class _FakeAgentApiClient extends AgentApiClient { @@ -290,6 +369,7 @@ class _FakeTerminalSocketSession extends TerminalSocketSession { final resizeCalls = >[]; final sentInputs = []; int disposeCount = 0; + Completer? disposeCompleter; Completer _connectCompleter = Completer(); void Function(String frame)? _onFrame; void Function()? _onDisconnected; @@ -341,6 +421,7 @@ class _FakeTerminalSocketSession extends TerminalSocketSession { @override Future dispose() async { disposeCount += 1; + await disposeCompleter?.future; } } @@ -366,3 +447,20 @@ class _FakeReconnectScheduler { } } } + +class _FakeResizeScheduler { + void Function()? pendingCallback; + + CancelReconnect schedule(Duration _, void Function() callback) { + pendingCallback = callback; + return () { + pendingCallback = null; + }; + } + + void runPending() { + final callback = pendingCallback; + pendingCallback = null; + callback?.call(); + } +}