diff --git a/app/lib/models/terminal_models.dart b/app/lib/models/terminal_models.dart index 6e22df2b..f02a0639 100644 --- a/app/lib/models/terminal_models.dart +++ b/app/lib/models/terminal_models.dart @@ -17,6 +17,23 @@ class TerminalTab { final String? type; // "agent" | "service" final bool unread; + /// Bumped whenever what the app knew about this PTY's geometry stops being + /// trustworthy — a reconnect, a same-id respawn, or a resize the service + /// queued and then discarded. + /// + /// The driver only re-sends `terminal:resize` when its computed grid differs + /// from the last size it believes the PTY received, so a size that never + /// arrived (a send dropped in a keyless window, or a queued one cancelled + /// before it reached the wire) and one a fresh PTY never had (a respawn takes + /// the bridge's `lastDriverGeometry`, which is whatever terminal resized + /// last — 80x24 only on a bridge that has never seen a resize) are both + /// disagreements nothing else can detect: the panel is not moving, so the + /// wrapper computes the same grid forever and the gate stays shut. The + /// counter is the invalidation edge that reopens it, invalidated by the same + /// events as the snapshot-seq cutoff in `TerminalService._rehydrateTerminals` + /// and for the same reason. + final int sizeEpoch; + /// Ghostty controller — the agent's PTY bytes are fed in via /// `ghostty.appendOutputBytes(...)` from terminal_service, and user /// input is routed back through `attachExternalTransport` to the @@ -34,6 +51,7 @@ class TerminalTab { this.exitCode, this.type, this.unread = false, + this.sizeEpoch = 0, GhosttyTerminalController? ghostty, }) : ghostty = ghostty ?? @@ -64,6 +82,7 @@ class TerminalTab { bool clearExitCode = false, String? type, bool? unread, + int? sizeEpoch, }) { return TerminalTab( terminalId: terminalId, @@ -78,6 +97,7 @@ class TerminalTab { exitCode: clearExitCode ? null : (exitCode ?? this.exitCode), type: type ?? this.type, unread: unread ?? this.unread, + sizeEpoch: sizeEpoch ?? this.sizeEpoch, ghostty: ghostty, ); } diff --git a/app/lib/services/terminal_service.dart b/app/lib/services/terminal_service.dart index 4a303c52..1302cd91 100644 --- a/app/lib/services/terminal_service.dart +++ b/app/lib/services/terminal_service.dart @@ -145,7 +145,30 @@ class TerminalService { // conditional on one would strand a cutoff above every seq a respawned PTY // emits and leave the pane blank behind a live process. _snapshotSeq.clear(); - for (final tab in _state.tabs.values) { + // The geometry is invalidated on the same grounds as the cutoff above: a + // reattach can hide a resize the agent never applied, and an + // exit-and-respawn nothing on the wire reported (the seq reasoning in the + // constructor). Neither is detectable from the app's side, and the driver's + // gate compares against what it BELIEVES it sent, so an unmoving panel + // never reopens it. Bumped for every live tab rather than only the + // re-pulled ones; the bridge folds a resize that changes nothing away, so a + // bump that proves unnecessary costs no SIGWINCH. + // + // The `focusResumed` caller keeps its transport, so nothing was lost there + // — but it shares this path because the same edge covers a heavy + // re-subscribe, which an unwitnessed respawn can hide just as a reconnect + // can. + final rehydrated = Map.from(_state.tabs); + var invalidated = false; + for (final entry in _state.tabs.entries) { + if (!_hasLivePty(entry.value)) continue; + rehydrated[entry.key] = entry.value.copyWith( + sizeEpoch: entry.value.sizeEpoch + 1, + ); + invalidated = true; + } + if (invalidated) _setState(_state.copyWith(tabs: rehydrated)); + for (final tab in rehydrated.values) { // A pending tab is the app's own optimistic invention — the agent has // never confirmed the id, so it would answer "snapshot requested for // unknown terminal" and send nothing. Its own terminal:started carries @@ -155,6 +178,33 @@ class TerminalService { } } + /// Whether a resize aimed at [tab] can reach a PTY. + /// + /// A pending id is the app's own optimistic invention the agent has never + /// confirmed, and an exited tab has no PTY behind it — an agent tab keeps + /// rendering the terminal view past its own exit, so a geometry + /// invalidation on either only buys a frame the bridge logs as unknown and + /// drops. + bool _hasLivePty(TerminalTab tab) => + tab.sessionState == TerminalSessionState.running && + !_pendingTerminalIds.contains(tab.terminalId); + + /// Retires whatever the driver believes [terminalId]'s geometry is, so its + /// next build re-sends at an unchanged panel size. + /// + /// Called wherever a resize the app already reported as accepted is known + /// not to have reached the PTY. `sendResize` answers at QUEUE time, and the + /// 100ms debounce it arms can still be cancelled or discarded afterwards; + /// the caller has booked the size by then, so nothing but an epoch bump + /// reopens its gate. + void _invalidateGeometry(String terminalId) { + final tab = _state.tabs[terminalId]; + if (tab == null) return; + final tabs = Map.from(_state.tabs); + tabs[terminalId] = tab.copyWith(sizeEpoch: tab.sizeEpoch + 1); + _setState(_state.copyWith(tabs: tabs)); + } + void _setState(TerminalState state) { if (_disposed) return; // Focusing a terminal — by ANY path (list tap, pinned/pushed view, agent @@ -353,6 +403,13 @@ class TerminalService { if (existing != null) { existing.ghostty.setSessionRunning(true); + // A start on an id the app already holds is a RESPAWN, and the new PTY + // carries whatever `TerminalManager.lastDriverGeometry` held — the size + // of whichever terminal resized last in that bridge process, or 80x24 on + // a bridge that has never seen a resize — not the one the driver sent the + // dead one. The driver gates its re-sends on the last size it believes + // the PTY has, so without this bump it computes the same grid, sees no + // change, and leaves a wide panel rendering a narrower process. tabs[msg.terminalId] = existing.copyWith( sessionState: TerminalSessionState.running, shell: msg.shell, @@ -360,6 +417,7 @@ class TerminalService { rows: msg.rows, clearExitCode: true, type: msg.terminalType, + sizeEpoch: existing.sizeEpoch + 1, ); } else { final tab = _createTab( @@ -401,8 +459,11 @@ class TerminalService { clientId == null || (msg.driverClientId != clientId && (pendingBase == null || msg.driverClientId != pendingBase)); + var dropped = false; if (stalePending) { - _resizeTimers.remove(msg.terminalId)?.cancel(); + final queued = _resizeTimers.remove(msg.terminalId); + queued?.cancel(); + dropped = queued != null; _resizeBaseDrivers.remove(msg.terminalId); } final tabs = Map.from(_state.tabs); @@ -410,6 +471,10 @@ class TerminalService { cols: msg.cols, rows: msg.rows, driverClientId: msg.driverClientId, + // The caller booked that size the moment `sendResize` queued it, so a + // frame cancelled here would otherwise leave its gate shut against a + // geometry the PTY never received. + sizeEpoch: dropped ? tab.sizeEpoch + 1 : null, ); _setState(_state.copyWith(tabs: tabs)); } @@ -468,6 +533,14 @@ class TerminalService { final existing = _state.tabs[info.terminalId]; if (existing != null) { existing.ghostty.setSessionRunning(info.running); + // The same respawn `_handleTerminalStarted` bumps on, seen through the + // status frame instead: a client that missed the live started frame + // builds its tabs from this replay (see the discovery pull below), so + // without the bump here the driver's booking survives a PTY that never + // received it — on the one path a relay app actually uses. + final respawned = + info.running && + existing.sessionState == TerminalSessionState.exited; final updated = existing.copyWith( name: info.name, sessionState: info.running @@ -477,6 +550,7 @@ class TerminalService { cols: info.cols, rows: info.rows, type: info.type, + sizeEpoch: respawned ? existing.sizeEpoch + 1 : null, ); newTabs[info.terminalId] = info.driverClientId == null ? updated.copyWith(clearDriverClientId: true) @@ -657,14 +731,27 @@ class TerminalService { sendInput(agentTabs.first.terminalId, text); } - void sendResize( + /// Queues a debounced `terminal:resize`, reporting whether it was QUEUED. + /// + /// False means nothing was queued and nothing ever will be for this call — + /// the per-install client id has not resolved yet (see + /// `terminalStateProvider`, which pushes it in), or the service is gone. The + /// caller must not record the size as sent: the wrapper gates re-sends on the + /// last size it believes the PTY has, so a drop booked as a send strands the + /// PTY at the previous geometry until the panel happens to change size again. + /// + /// True is not a delivery receipt. The 100ms debounce this arms can still be + /// cancelled (`_handleTerminalSize`, `deleteTerminal`) or discarded by its own + /// driver guard, and every such path owes the caller an `_invalidateGeometry` + /// — that bump is what retires a booking the wire never honoured. + bool sendResize( String terminalId, int cols, int rows, { String? baseDriverClientId, }) { final clientId = _clientId; - if (clientId == null) return; + if (_disposed || clientId == null) return false; _resizeTimers[terminalId]?.cancel(); _resizeBaseDrivers[terminalId] = baseDriverClientId; _resizeTimers[terminalId] = Timer(const Duration(milliseconds: 100), () { @@ -675,6 +762,9 @@ class TerminalService { currentDriver != null && currentDriver != baseDriverClientId && currentDriver != clientId) { + // Discarded, not sent — and the caller booked this size when the queue + // accepted it, so hand back the invalidation edge that reopens its gate. + _invalidateGeometry(terminalId); return; } _send( @@ -687,6 +777,7 @@ class TerminalService { }), ); }); + return true; } void requestStart( diff --git a/app/lib/widgets/terminal_cell_metrics.dart b/app/lib/widgets/terminal_cell_metrics.dart index 946a431b..f1419ba9 100644 --- a/app/lib/widgets/terminal_cell_metrics.dart +++ b/app/lib/widgets/terminal_cell_metrics.dart @@ -16,11 +16,13 @@ const double kGhosttyLineHeight = 1.35; /// one frame LATE, and on every remount (a session switch re-keys the wrapper) /// that first frame has no metrics at all. Laying the grid out at a /// locally-derived width for that one frame and then correcting it is a real -/// grid resize under the guest, and `ghostty_vte_flutter` does not reflow: an -/// Ink-style TUI leaks stale fragments across it. +/// grid resize under the guest, and an Ink-style TUI leaks stale fragments +/// across it. The engine's own reflow does not save this: it re-wraps SOFT +/// wrapped rows only, and a TUI that wraps its own output writes the break +/// itself — see `terminal_reflow_contract_test.dart`, which pins both halves. /// /// HAND-MIRRORED against the pinned fork (`ghostty_vte_flutter`, ref -/// `c262d5f2002d26b2116b2c5c943a46a63f994133`): +/// `6831ba09fe9a298ecd9e1d9bdb40de141b9bfbee`): /// `_GhosttyTerminalViewState._measureMetrics` and /// `_snapLogicalExtentToPhysical` in `lib/src/terminal_view.dart`. The package /// defaults the wrapper does not override are folded in — `cellWidthScale = 1`, diff --git a/app/lib/widgets/terminal_view_wrapper.dart b/app/lib/widgets/terminal_view_wrapper.dart index 36b72188..d601fcb4 100644 --- a/app/lib/widgets/terminal_view_wrapper.dart +++ b/app/lib/widgets/terminal_view_wrapper.dart @@ -235,6 +235,10 @@ class _TerminalViewWrapperState extends ConsumerState { int? _lastSentCols; int? _lastSentRows; + /// The `TerminalTab.sizeEpoch` [_lastSentCols]/[_lastSentRows] were booked + /// against, so a bump can retire them. + int? _observedSizeEpoch; + /// Size the driver's grid is actually rendered at, reported by /// `_TerminalGridFreeze`. The PTY's authoritative `cols`/`rows` are derived /// from this (not the live viewport) so the size sent to the PTY lands in @@ -336,6 +340,38 @@ class _TerminalViewWrapperState extends ConsumerState { }); } + @override + void didUpdateWidget(TerminalViewWrapper oldWidget) { + super.didUpdateWidget(oldWidget); + // The booking below is per-PTY, but this State is not: only + // `terminal_screen` keys the wrapper by terminalId — the pinned pane + // (`terminal_list_view`), `terminal_detail_view` and the setup banner all + // mount it unkeyed, so swapping which terminal they show reuses this State. + // Carried over, `_lastSent*` gate the new PTY against the old one's grid, + // and two tabs sitting at the same epoch (the common case — both 0, or both + // bumped in lockstep by a reattach) make the bump no edge at all. That is + // the exact stranding `sizeEpoch` exists to prevent, reached through the + // widget tree instead of the wire. + if (oldWidget.tab.terminalId == widget.tab.terminalId) return; + _lastSentCols = null; + _lastSentRows = null; + _observedSizeEpoch = null; + // A pointer-down authorizes taking width ownership of the terminal the + // user pressed, not of whichever one lands in this slot next. + _claimRequestedByUser = false; + // Both halves of the claim gate, because the swap fires no focus event to + // re-arm the other one: `_claimed` is cleared only on a focus GAIN, and + // these slots swap while the wrapper keeps focus. On a touch device that is + // terminal — `_requestUserClaim` returns immediately without a physical + // keyboard, so `autoClaiming` is the only claim path there, and a `true` + // carried over from the previous terminal leaves the phone letterboxed at + // another device's grid with no gesture that takes it back. + _claimed = false; + // `_renderSize` is deliberately kept: it describes the PANEL's settled + // grid, which this swap does not move, and `_TerminalGridFreeze` survives + // the slot too — so it would never be re-reported if it were cleared. + } + late final ProviderContainer _container; /// The exact callback published to [focusAgentInputProvider], held so dispose @@ -869,6 +905,7 @@ class _TerminalViewWrapperState extends ConsumerState { constraints, amDriver, tab.driverClientId, + sizeEpoch: tab.sizeEpoch, charWidth: cell.charWidth, lineHeightPx: cell.linePixels, ); @@ -876,10 +913,13 @@ class _TerminalViewWrapperState extends ConsumerState { // Driver → fill the viewport. Pin the grid to the last // settled width via `_TerminalGridFreeze` so transient // resizes (e.g. dragging the agent/workspace divider) - // don't spam Ghostty grid resizes — - // `ghostty_vte_flutter` doesn't reflow soft-wrapped lines, and - // Ink-style TUI redraws (Claude Code) leak stale fragments - // when the grid changes underneath them. + // don't spam Ghostty grid resizes — Ink-style TUI + // redraws (Claude Code) leak stale fragments when the + // grid changes underneath them. The engine DOES reflow + // its own soft-wrapped rows, but that reaches none of + // this: such a TUI wraps its output itself, so every row + // it wrote is a hard break reflow cannot re-join + // (`terminal_reflow_contract_test.dart`). if (amDriver) { return _TerminalGridFreeze( onSettled: _onRenderSizeSettled, @@ -1036,9 +1076,20 @@ class _TerminalViewWrapperState extends ConsumerState { BoxConstraints constraints, bool amDriver, String? observedDriverClientId, { + required int sizeEpoch, required double charWidth, required double lineHeightPx, }) { + // A bump means the PTY no longer holds what we booked — it respawned, or a + // send the service accepted never reached the wire (`TerminalTab.sizeEpoch` + // names every case). Reopening the `changed` gate is the only thing that + // recovers it: the panel is not moving, so every later build computes the + // same grid and would return below forever. + // + // Read here, retired only where the booking it belongs to is written — a + // build that computes a grid is not a build that sends one, and consuming + // the edge during layout spends it on the early returns too. + final epochStale = _observedSizeEpoch != sizeEpoch; // Subtract the view's symmetric padding from BOTH axes so the native grid // matches what GhosttyTerminalView._syncGrid actually renders // (floor((dim - padding) / cellMetric)). `_hPad` (= padding.horizontal, @@ -1076,23 +1127,40 @@ class _TerminalViewWrapperState extends ConsumerState { final autoClaiming = !_hasPhysicalKeyboard && _locallyActive && !_claimed; final userClaiming = _claimRequestedByUser && !_claimed; final claiming = autoClaiming || userClaiming; - final changed = nativeCols != _lastSentCols || nativeRows != _lastSentRows; + final changed = + epochStale || + nativeCols != _lastSentCols || + nativeRows != _lastSentRows; if (!claiming && !(amDriver && changed)) return; WidgetsBinding.instance.addPostFrameCallback((_) { if (!mounted) return; + // Book the size and consume the claim only if the service actually took + // the request. A terminal can mount before the per-install client id + // resolves, and `sendResize` drops those; recording them anyway leaves + // `changed` false against a size the PTY never received, so the agent + // keeps wrapping at the old width until the panel happens to move again. + // A rejected attempt leaves `changed` true, and this view watches + // `clientIdProvider` itself (`_buildTerminal`), so the id landing rebuilds + // it — which is what retries the send. + // + // Queued is not delivered: `sendResize` can still discard the frame it + // armed, and those paths answer with a `sizeEpoch` bump rather than a + // return value, because they resolve long after this callback is gone. + final sent = widget.terminalService.sendResize( + terminalId, + nativeCols, + nativeRows, + baseDriverClientId: observedDriverClientId, + ); + if (!sent) return; if (claiming) { _claimed = true; _claimRequestedByUser = false; } + _observedSizeEpoch = sizeEpoch; _lastSentCols = nativeCols; _lastSentRows = nativeRows; - widget.terminalService.sendResize( - terminalId, - nativeCols, - nativeRows, - baseDriverClientId: observedDriverClientId, - ); }); } @@ -1185,12 +1253,30 @@ class _TerminalGridFreezeState extends State<_TerminalGridFreeze> { Size? _pinnedSize; Timer? _settleTimer; + /// The size [_settleTimer] is currently counting down towards, so a rebuild + /// that observes the same size does not restart it. + Size? _settlingTo; + @override void dispose() { - _settleTimer?.cancel(); + _cancelSettle(); super.dispose(); } + /// Whether two observed sizes are the same to the only precision that + /// matters here — the integer cell grid the pin protects. + /// + /// Not `Size ==`: that is exact double equality, and a width arriving as + /// 599.9999999999999 on one layout pass and 600.0 on the next (a fractional + /// flex split, a divider still animating toward its endpoint) would read as + /// movement. The guards below would then re-arm on every rebuild and restore + /// the never-settles behaviour they exist to remove, with nothing visible to + /// say why. + static bool _sameSize(Size? a, Size b) => + a != null && + (a.width - b.width).abs() < 0.5 && + (a.height - b.height).abs() < 0.5; + /// Notify the parent of the rendered size without mutating state during /// layout (the immediate pin happens inside `build`): defer to post-frame. void _notifySettled(Size size) { @@ -1201,15 +1287,38 @@ class _TerminalGridFreezeState extends State<_TerminalGridFreeze> { }); } + /// Arms the settle countdown for [size], measuring quiet from the last time + /// the size actually MOVED — never from the last rebuild. + /// + /// `LayoutBuilder` re-runs its builder whenever the parent rebuilds, not only + /// when constraints change, so re-arming unconditionally makes any wrapper + /// rebuild faster than [_settleDelay] (a streaming agent, a selection drag, a + /// session-list tick) cancel the timer forever. The grid then stays pinned to + /// whatever it held when the panel last changed size — the content clipped at + /// the stale column with dead space beside it, for as long as the rebuilds + /// keep coming. void _scheduleSettle(Size size) { + if (_sameSize(_settlingTo, size)) return; + _settlingTo = size; _settleTimer?.cancel(); _settleTimer = Timer(_settleDelay, () { + // Cleared before the mount check so `_settlingTo` never outlives the + // timer it names — every other guard here reads it as "a countdown is + // running". + _settlingTo = null; + _settleTimer = null; if (!mounted) return; setState(() => _pinnedSize = size); _notifySettled(size); }); } + void _cancelSettle() { + _settleTimer?.cancel(); + _settleTimer = null; + _settlingTo = null; + } + @override Widget build(BuildContext context) { return LayoutBuilder( @@ -1221,8 +1330,13 @@ class _TerminalGridFreezeState extends State<_TerminalGridFreeze> { _pinnedSize = live; _notifySettled(live); } - if (_pinnedSize != live) { + if (!_sameSize(_pinnedSize, live)) { _scheduleSettle(live); + } else { + // Back at the pinned size before the countdown expired (a drag that + // returned, a transient constraint). Dropping the timer keeps it from + // pinning a size the panel no longer has. + _cancelSettle(); } final inner = _pinnedSize!; return ClipRect( diff --git a/app/test/services/terminal_reattach_test.dart b/app/test/services/terminal_reattach_test.dart index b358e0bd..e46e766b 100644 --- a/app/test/services/terminal_reattach_test.dart +++ b/app/test/services/terminal_reattach_test.dart @@ -408,4 +408,55 @@ void main() { await session.close(); }); + + // The PTY's geometry is invalidated by exactly the two events the seq cutoff + // is: a re-drive and a same-id respawn. The driver re-sends `terminal:resize` + // only when its computed grid differs from the last size it believes the PTY + // received, so neither event has any other way to reach it — the panel is + // not moving, so the wrapper keeps computing the same grid and the gate stays + // shut for as long as the terminal is on screen. + test('a re-drive retires the geometry the driver booked', () async { + if (_skipWithoutNative()) return; + final t = FakeAgentTransport(); + final session = await makeSession(t); + await seedTabA(t); + final before = session.terminalService.currentState.tabs['a']!.sizeEpoch; + + t.redriveHydrators(); + await Future.delayed(Duration.zero); + + expect( + session.terminalService.currentState.tabs['a']!.sizeEpoch, + greaterThan(before), + reason: 'a resize sent while the stream was away vanished unreported', + ); + + await session.close(); + }); + + test('a same-id respawn retires the geometry the driver booked', () async { + if (_skipWithoutNative()) return; + final t = FakeAgentTransport(); + final session = await makeSession(t); + await seedTabA(t); + final before = session.terminalService.currentState.tabs['a']!.sizeEpoch; + + // A fresh PTY on a known id. Its geometry is the bridge's, not whatever + // the driver had sent the process that just died — `lastDriverGeometry` if + // any terminal has resized in that bridge process, 80x24 (used here) if + // none has. + t.emit('terminal:started', { + 'terminalId': 'a', + 'shell': 'bash', + 'cols': 80, + 'rows': 24, + }); + await Future.delayed(Duration.zero); + + final tab = session.terminalService.currentState.tabs['a']!; + expect(tab.cols, 80); + expect(tab.sizeEpoch, greaterThan(before)); + + await session.close(); + }); } diff --git a/app/test/services/terminal_size_service_test.dart b/app/test/services/terminal_size_service_test.dart index a5d29523..750ad9a4 100644 --- a/app/test/services/terminal_size_service_test.dart +++ b/app/test/services/terminal_size_service_test.dart @@ -201,4 +201,119 @@ void main() { await svc.dispose(); await session.close(); }); + + // The two paths where `sendResize` returns true and the frame it armed is + // then thrown away. The caller books the size on that `true`, so its gate is + // shut against a geometry the PTY never received; only a `sizeEpoch` bump + // reopens it, and nothing else in the system can detect the disagreement. + test('a queued resize discarded by another driver bumps sizeEpoch', () async { + final t = FakeAgentTransport(); + final session = await newSession(t); + final svc = TerminalService.fromSession(session); + svc.setClientId('desktop'); + + t.emit('agent:status', { + 'projectId': 'p', + 'terminals': [ + { + 'id': 't1', + 'terminalId': 't1', + 'name': 'Terminal 1', + 'running': true, + 'cols': 120, + 'rows': 30, + 'driverClientId': 'desktop', + }, + ], + }); + await Future.delayed(Duration.zero); + final before = svc.currentState.tabs['t1']!.sizeEpoch; + + // Nothing queued: the frame is authoritative news, not a cancellation, and + // the caller has no booking to retire. + t.emit('terminal:size', { + 'terminalId': 't1', + 'cols': 90, + 'rows': 30, + 'driverClientId': 'mobile', + }); + await Future.delayed(Duration.zero); + expect(svc.currentState.tabs['t1']!.sizeEpoch, before); + + expect( + svc.sendResize('t1', 121, 30, baseDriverClientId: 'desktop'), + isTrue, + ); + t.emit('terminal:size', { + 'terminalId': 't1', + 'cols': 50, + 'rows': 40, + 'driverClientId': 'mobile', + }); + await Future.delayed(const Duration(milliseconds: 150)); + + expect(t.sent.where((m) => m['type'] == 'terminal:resize'), isEmpty); + expect( + svc.currentState.tabs['t1']!.sizeEpoch, + before + 1, + reason: 'the queued frame was destroyed, so the booking must be retired', + ); + + await svc.dispose(); + await session.close(); + }); + + test('a resize the debounce itself discards bumps sizeEpoch', () async { + final t = FakeAgentTransport(); + final session = await newSession(t); + final svc = TerminalService.fromSession(session); + svc.setClientId('mobile'); + + t.emit('agent:status', { + 'projectId': 'p', + 'terminals': [ + { + 'id': 't1', + 'terminalId': 't1', + 'name': 'Terminal 1', + 'running': true, + 'cols': 120, + 'rows': 30, + 'driverClientId': 'desktop', + }, + ], + }); + await Future.delayed(Duration.zero); + final before = svc.currentState.tabs['t1']!.sizeEpoch; + + expect(svc.sendResize('t1', 50, 40, baseDriverClientId: 'desktop'), isTrue); + // A THIRD device takes the terminal, announced on the status tier rather + // than as a `terminal:size` — so the cancellation above never runs and the + // armed timer survives to reject itself against the new driver. + t.emit('agent:status', { + 'projectId': 'p', + 'terminals': [ + { + 'id': 't1', + 'terminalId': 't1', + 'name': 'Terminal 1', + 'running': true, + 'cols': 120, + 'rows': 30, + 'driverClientId': 'tablet', + }, + ], + }); + await Future.delayed(const Duration(milliseconds: 150)); + + expect(t.sent.where((m) => m['type'] == 'terminal:resize'), isEmpty); + expect( + svc.currentState.tabs['t1']!.sizeEpoch, + before + 1, + reason: 'the debounce dropped the frame long after sendResize answered', + ); + + await svc.dispose(); + await session.close(); + }); } diff --git a/app/test/widgets/terminal_letterbox_test.dart b/app/test/widgets/terminal_letterbox_test.dart index 09dab577..be323478 100644 --- a/app/test/widgets/terminal_letterbox_test.dart +++ b/app/test/widgets/terminal_letterbox_test.dart @@ -7,6 +7,8 @@ // applies to BOTH axes; a grid that fits is letterboxed (centered) at its // natural size, since the scale never enlarges. Separately, a view that // never gains focus must never claim the driver role by sending a resize. +import 'dart:async'; + import 'package:antgrid/models/terminal_models.dart'; import 'package:antgrid/project/project_session.dart'; import 'package:antgrid/providers/client_id.dart'; @@ -92,9 +94,14 @@ TerminalTab _tab({ return tab; } -Widget _wrap(Widget child) => ProviderScope( +/// [clientId] lets a test hold the provider in `AsyncLoading`: the real one +/// reads SharedPreferences, so a terminal can be on screen before it resolves, +/// and the load→data transition is the rebuild the wrapper's retry depends on. +Widget _wrap(Widget child, {Future? clientId}) => ProviderScope( overrides: [ - clientIdProvider.overrideWith((ref) async => _myClientId), + clientIdProvider.overrideWith( + (ref) => clientId ?? Future.value(_myClientId), + ), // _buildTerminal watches agentTerminalProvider for the send-to-agent // overlay; these tabs are not the agent, so pin it null to keep the // throwing focused-session façades out of the test. @@ -122,6 +129,11 @@ void main() { _settingsPrefs = await openAppSettingsPrefs(); }); + // Every test here overrides the platform and clears it as its last statement, + // which a failing `expect` skips — leaking the override into every test after + // it and turning one real failure into a cascade of platform-dependent ones. + tearDown(() => debugDefaultTargetPlatformOverride = null); + testWidgets( 'non-driver grid larger than the viewport is scaled down, not scrolled', (tester) async { @@ -442,4 +454,224 @@ void main() { debugDefaultTargetPlatformOverride = null; }); + + testWidgets( + 'driver grid settles to a grown panel while the wrapper keeps rebuilding', + (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.macOS; + final h = await _makeService(addTearDown); + h.service.setClientId(_myClientId); + + // The grid-freeze delay measures quiet on the SIZE, not on rebuilds: + // LayoutBuilder re-runs its builder on every parent rebuild, so re-arming + // per rebuild lets a wrapper rebuilding faster than the delay (a + // streaming agent) hold the timer off forever and strand the grid at the + // pre-resize width — content clipped at the stale column with dead space + // beside it. + final tab = _tab(id: 't8', cols: 80, driverClientId: _myClientId); + + final width = ValueNotifier(300); + final rebuilds = ValueNotifier(0); + addTearDown(() { + width.dispose(); + rebuilds.dispose(); + }); + + await tester.pumpWidget( + _wrap( + AnimatedBuilder( + animation: Listenable.merge([width, rebuilds]), + builder: (context, _) => Align( + alignment: Alignment.topLeft, + child: SizedBox( + width: width.value, + height: 400, + child: TerminalViewWrapper( + tab: tab, + terminalService: h.service, + ), + ), + ), + ), + ), + ); + await tester.pumpAndSettle(); + expect( + tester.getSize(find.byType(GhosttyTerminalView)).width, + closeTo(300, _epsilon), + ); + + width.value = 600; + for (var i = 0; i < 12; i++) { + rebuilds.value++; + await tester.pump(const Duration(milliseconds: 50)); + } + + expect( + tester.getSize(find.byType(GhosttyTerminalView)).width, + closeTo(600, _epsilon), + ); + + debugDefaultTargetPlatformOverride = null; + }, + ); + + testWidgets( + 'a resize dropped for a not-yet-resolved client id is retried, not booked', + (tester) async { + debugDefaultTargetPlatformOverride = TargetPlatform.macOS; + final h = await _makeService(addTearDown); + // Deliberately unresolved: the terminal is on screen before the + // per-install id is read off disk, which is the startup order the wrapper + // has to survive. `sendResize` drops those, and booking one as sent would + // leave the PTY at its spawn geometry with nothing left to trigger a + // re-send. + final clientId = Completer(); + final tab = _tab(id: 't9', cols: 80, driverClientId: null); + + // Built ONCE. Completing the id below is the only thing that rebuilds + // this tree, so the test fails if the wrapper stops watching + // `clientIdProvider` — pumping a second tree by hand would supply the + // rebuild the production path is supposed to provide for itself. + await tester.pumpWidget( + _wrap( + SizedBox( + width: 300, + height: 400, + child: TerminalViewWrapper(tab: tab, terminalService: h.service), + ), + clientId: clientId.future, + ), + ); + await tester.pumpAndSettle(); + await tester.pump(const Duration(milliseconds: 150)); + expect( + h.transport.sent.where((m) => m['type'] == 'terminal:resize'), + isEmpty, + reason: 'no client id yet, so nothing can be stamped and sent', + ); + + // The id lands: the size is still unsent, so the same geometry must go + // out now rather than wait for a panel resize. + h.service.setClientId(_myClientId); + clientId.complete(_myClientId); + await tester.pumpAndSettle(); + await tester.pump(const Duration(milliseconds: 150)); + + final resizes = h.transport.sent + .where((m) => m['type'] == 'terminal:resize') + .toList(); + expect(resizes, isNotEmpty); + expect(resizes.last['clientId'], _myClientId); + + debugDefaultTargetPlatformOverride = null; + }, + ); + + testWidgets('a sizeEpoch bump reopens the resize gate at an unchanged size', ( + tester, + ) async { + debugDefaultTargetPlatformOverride = TargetPlatform.macOS; + final h = await _makeService(addTearDown); + h.service.setClientId(_myClientId); + // Drives its own PTY, so the driver branch is live and the settled size is + // what the resize is derived from. + var tab = _tab(id: 't10', cols: 80, driverClientId: _myClientId); + + Future show() async { + await tester.pumpWidget( + _wrap( + SizedBox( + width: 500, + height: 400, + child: TerminalViewWrapper(tab: tab, terminalService: h.service), + ), + ), + ); + await tester.pumpAndSettle(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pumpAndSettle(); + } + + await show(); + expect( + h.transport.sent.where((m) => m['type'] == 'terminal:resize'), + isNotEmpty, + ); + + // Panel unchanged: the booked size still stands, so nothing goes out. This + // is the gate that strands a respawned PTY, and it has to be shut here for + // the bump below to prove anything. + h.transport.sent.clear(); + await show(); + expect( + h.transport.sent.where((m) => m['type'] == 'terminal:resize'), + isEmpty, + ); + + // The service reports the geometry as no longer trustworthy — a re-drive + // or a respawn. The panel STILL has not moved, so the bump is the only + // thing that can put the size back on the wire. + tab = tab.copyWith(sizeEpoch: tab.sizeEpoch + 1); + await show(); + + final resizes = h.transport.sent + .where((m) => m['type'] == 'terminal:resize') + .toList(); + expect(resizes, isNotEmpty); + expect(resizes.last['clientId'], _myClientId); + + debugDefaultTargetPlatformOverride = null; + }); + + testWidgets('swapping which terminal an unkeyed wrapper shows re-sends', ( + tester, + ) async { + debugDefaultTargetPlatformOverride = TargetPlatform.macOS; + final h = await _makeService(addTearDown); + h.service.setClientId(_myClientId); + + // Only `terminal_screen` keys the wrapper by terminalId; the pinned pane, + // the detail view and the setup banner all mount it unkeyed, so this swap + // REUSES one State. Both tabs drive, sit at the same epoch and are shown at + // the same panel size — so every gate the wrapper carries per-PTY reads as + // "already sent" unless the swap itself retires them. + final a = _tab(id: 'swap-a', cols: 80, driverClientId: _myClientId); + final b = _tab(id: 'swap-b', cols: 80, driverClientId: _myClientId); + + Future show(TerminalTab tab) async { + await tester.pumpWidget( + _wrap( + SizedBox( + width: 500, + height: 400, + child: TerminalViewWrapper(tab: tab, terminalService: h.service), + ), + ), + ); + await tester.pumpAndSettle(); + await tester.pump(const Duration(milliseconds: 300)); + await tester.pumpAndSettle(); + } + + await show(a); + expect( + h.transport.sent.where((m) => m['type'] == 'terminal:resize'), + isNotEmpty, + ); + + h.transport.sent.clear(); + await show(b); + + final resizes = h.transport.sent + .where((m) => m['type'] == 'terminal:resize') + .toList(); + expect( + resizes.map((m) => m['terminalId']), + contains('swap-b'), + reason: "the new PTY has never been told this panel's grid", + ); + + debugDefaultTargetPlatformOverride = null; + }); } diff --git a/app/test/widgets/terminal_reflow_contract_test.dart b/app/test/widgets/terminal_reflow_contract_test.dart new file mode 100644 index 00000000..6d88dfbd --- /dev/null +++ b/app/test/widgets/terminal_reflow_contract_test.dart @@ -0,0 +1,93 @@ +// Pins what the pinned `ghostty_vte_flutter` engine does to already-written +// rows when the column count changes. +// +// Load-bearing because `_TerminalGridFreeze` (terminal_view_wrapper.dart) rests +// entirely on the second half of that contract. The engine re-wraps rows IT +// soft-wrapped, so reflow alone would make a grid resize harmless — but a row +// the guest broke itself is a hard break no reflow can re-join, and an +// Ink-style TUI wraps its own output, so every row it writes is in that second +// class. That is why a grid resized underneath one leaks stale fragments and +// why the pin exists. A failure in the first test means the engine stopped +// reflowing; a failure in the second means the freeze's rationale went with it. +import 'package:flutter_test/flutter_test.dart'; +import 'package:ghostty_vte_flutter/ghostty_vte_flutter.dart'; + +/// True when the native VT is missing, having marked the current test skipped. +/// +/// Skipped rather than quietly returned from: a bare early return makes a host +/// with no prebuilt libghostty-vt report a green suite that asserted nothing. +bool _skipWithoutNative() { + if (_hasNative()) return false; + markTestSkipped('native VT unavailable'); + return true; +} + +bool _hasNative() { + try { + GhosttyVt.newTerminal(cols: 8, rows: 2).close(); + return true; + } catch (_) { + return false; + } +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + test('a soft-wrapped row re-wraps when the grid widens', () { + if (_skipWithoutNative()) return; + final c = GhosttyTerminalController(); + addTearDown(c.dispose); + c.attachExternalTransport(writeBytes: (_) => true); + c.resize(cols: 40, rows: 10); + + // 60 printable chars with no newline: the engine soft-wraps it at 40. + final text = List.generate( + 60, + (i) => String.fromCharCode(97 + i % 26), + ).join(); + c.appendOutputBytes(text.codeUnits); + + final narrow = c.lines.where((l) => l.trim().isNotEmpty).toList(); + expect(narrow.length, 2, reason: 'soft-wrapped into two rows at 40 cols'); + + c.resize(cols: 80, rows: 10); + final wide = c.lines.where((l) => l.trim().isNotEmpty).toList(); + printOnFailure('narrow: $narrow\nwide: $wide'); + + expect( + wide.length, + 1, + reason: 'the engine reflows the primary screen when the grid widens', + ); + expect(wide.single.trimRight(), text); + }); + + test('a HARD-wrapped row does not re-join when the grid widens', () { + if (_skipWithoutNative()) return; + final c = GhosttyTerminalController(); + addTearDown(c.dispose); + c.attachExternalTransport(writeBytes: (_) => true); + c.resize(cols: 40, rows: 10); + + // What an Ink-style TUI emits: it wraps its own output AT the margin and + // writes the break itself. Each row is exactly `cols` wide, so it is + // indistinguishable from a soft wrap by width alone — the engine has to be + // tracking the wrap flag to keep them apart, which is the whole contract. + // Rows narrower than the margin would pass this test against an engine with + // no wrap tracking at all. + final a = 'a' * 40; + final b = 'b' * 40; + c.appendOutputBytes('$a\r\n$b'.codeUnits); + + final narrow = c.lines.where((l) => l.trim().isNotEmpty).toList(); + expect(narrow.length, 2, reason: 'two margin-filling rows at 40 cols'); + + c.resize(cols: 80, rows: 10); + final wide = c.lines.where((l) => l.trim().isNotEmpty).toList(); + printOnFailure('narrow: $narrow\nwide: $wide'); + + expect(wide.length, 2, reason: 'a self-written break is never re-joined'); + expect(wide.first.trimRight(), a); + }); +} diff --git a/app/test/widgets/terminal_remount_test.dart b/app/test/widgets/terminal_remount_test.dart index 0c27a157..d2c8f6e3 100644 --- a/app/test/widgets/terminal_remount_test.dart +++ b/app/test/widgets/terminal_remount_test.dart @@ -7,9 +7,11 @@ // corrected it to the driver's authoritative width — two engine resizes where // the authoritative width never moved, plus a runtimeType swap at the same tree // position that disposed the whole `GhosttyTerminalView` state. Neither is -// cosmetic: `ghostty_vte_flutter` does not reflow, so an Ink-style TUI leaks -// stale fragments across a grid change, which is what "returning to a terminal -// shows blank regions" looked like from the user's side. +// cosmetic: the engine reflows only the rows IT soft-wrapped, and an Ink-style +// TUI writes its own breaks, so such a TUI leaks stale fragments across a grid +// change (`terminal_reflow_contract_test.dart` pins both halves) — which is what +// "returning to a terminal shows blank regions" looked like from the user's +// side. import 'package:antgrid/design/ab_tokens.dart'; import 'package:antgrid/design/theme_presets.dart'; import 'package:antgrid/models/terminal_models.dart';