Fix terminal session resume reconnect
This commit is contained in:
parent
7ac7965e7e
commit
d7b005e09f
@ -31,7 +31,8 @@ class TerminalPage extends ConsumerStatefulWidget {
|
|||||||
ConsumerState<TerminalPage> createState() => _TerminalPageState();
|
ConsumerState<TerminalPage> createState() => _TerminalPageState();
|
||||||
}
|
}
|
||||||
|
|
||||||
class _TerminalPageState extends ConsumerState<TerminalPage> {
|
class _TerminalPageState extends ConsumerState<TerminalPage>
|
||||||
|
with WidgetsBindingObserver {
|
||||||
static const List<_QuickTerminalKey> _quickTerminalKeys = [
|
static const List<_QuickTerminalKey> _quickTerminalKeys = [
|
||||||
_QuickTerminalKey(keyId: 'esc', label: 'Esc', input: '\u001b'),
|
_QuickTerminalKey(keyId: 'esc', label: 'Esc', input: '\u001b'),
|
||||||
_QuickTerminalKey(keyId: 'tab', label: 'Tab', input: '\t'),
|
_QuickTerminalKey(keyId: 'tab', label: 'Tab', input: '\t'),
|
||||||
@ -57,6 +58,7 @@ class _TerminalPageState extends ConsumerState<TerminalPage> {
|
|||||||
@override
|
@override
|
||||||
void initState() {
|
void initState() {
|
||||||
super.initState();
|
super.initState();
|
||||||
|
WidgetsBinding.instance.addObserver(this);
|
||||||
_coordinator = TerminalSessionCoordinator(
|
_coordinator = TerminalSessionCoordinator(
|
||||||
controller: controller,
|
controller: controller,
|
||||||
apiClient: ref.read(agentApiClientProvider),
|
apiClient: ref.read(agentApiClientProvider),
|
||||||
@ -92,6 +94,7 @@ class _TerminalPageState extends ConsumerState<TerminalPage> {
|
|||||||
|
|
||||||
@override
|
@override
|
||||||
void dispose() {
|
void dispose() {
|
||||||
|
WidgetsBinding.instance.removeObserver(this);
|
||||||
_terminalFocusNode.dispose();
|
_terminalFocusNode.dispose();
|
||||||
_inputController.dispose();
|
_inputController.dispose();
|
||||||
_terminalScrollController.dispose();
|
_terminalScrollController.dispose();
|
||||||
@ -100,6 +103,16 @@ class _TerminalPageState extends ConsumerState<TerminalPage> {
|
|||||||
super.dispose();
|
super.dispose();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@override
|
||||||
|
void didChangeAppLifecycleState(AppLifecycleState state) {
|
||||||
|
if (state != AppLifecycleState.resumed) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
_diagnosticLog.add('app.lifecycle.resumed', widget.session.sessionId);
|
||||||
|
unawaited(_coordinator.reconnectNow());
|
||||||
|
}
|
||||||
|
|
||||||
Future<void> _openSiblingTerminal() async {
|
Future<void> _openSiblingTerminal() async {
|
||||||
final project = widget.project;
|
final project = widget.project;
|
||||||
if (project == null) {
|
if (project == null) {
|
||||||
|
|||||||
@ -527,6 +527,41 @@ void main() {
|
|||||||
expect(find.text('codex-main'), findsOneWidget);
|
expect(find.text('codex-main'), findsOneWidget);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
testWidgets('terminal page reconnects when the app resumes', (tester) async {
|
||||||
|
final transportFactory = _QueuedTerminalSocketTransportFactory();
|
||||||
|
|
||||||
|
await _pumpApp(
|
||||||
|
tester,
|
||||||
|
projectRepository: _FakeProjectRepository(),
|
||||||
|
sessionRepository: _FakeSessionRepository(),
|
||||||
|
socketFactory: TerminalSocketSessionFactory(
|
||||||
|
transportFactory: transportFactory.create,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
await _openProjectTerminal(tester);
|
||||||
|
|
||||||
|
expect(transportFactory.createCount, 1);
|
||||||
|
|
||||||
|
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.inactive);
|
||||||
|
await tester.pump();
|
||||||
|
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.hidden);
|
||||||
|
await tester.pump();
|
||||||
|
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.paused);
|
||||||
|
await tester.pump();
|
||||||
|
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.hidden);
|
||||||
|
await tester.pump();
|
||||||
|
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.inactive);
|
||||||
|
await tester.pump();
|
||||||
|
tester.binding.handleAppLifecycleStateChanged(AppLifecycleState.resumed);
|
||||||
|
await tester.pump();
|
||||||
|
await tester.pump(const Duration(seconds: 2));
|
||||||
|
await tester.pumpAndSettle();
|
||||||
|
|
||||||
|
expect(transportFactory.createCount, 2);
|
||||||
|
expect(find.text('codex-main'), findsOneWidget);
|
||||||
|
});
|
||||||
|
|
||||||
testWidgets('terminal page can open another terminal for the same project', (
|
testWidgets('terminal page can open another terminal for the same project', (
|
||||||
tester,
|
tester,
|
||||||
) async {
|
) async {
|
||||||
|
|||||||
@ -62,6 +62,15 @@ internal sealed class ConPtyProcessSession : IConPtySession
|
|||||||
await Task.CompletedTask.ConfigureAwait(false);
|
await Task.CompletedTask.ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public ValueTask<bool> IsAliveAsync(CancellationToken cancellationToken)
|
||||||
|
{
|
||||||
|
cancellationToken.ThrowIfCancellationRequested();
|
||||||
|
var isAlive = !_disposed &&
|
||||||
|
!_shutdown.IsCancellationRequested &&
|
||||||
|
TryGetExitCode() is null;
|
||||||
|
return ValueTask.FromResult(isAlive);
|
||||||
|
}
|
||||||
|
|
||||||
public async Task WriteInputAsync(string input, CancellationToken cancellationToken)
|
public async Task WriteInputAsync(string input, CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
ThrowIfDisposed();
|
ThrowIfDisposed();
|
||||||
|
|||||||
@ -108,6 +108,17 @@ internal sealed class HelperBackedConPtySession : IConPtySession
|
|||||||
_started = true;
|
_started = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public ValueTask<bool> IsAliveAsync(CancellationToken cancellationToken)
|
||||||
|
{
|
||||||
|
cancellationToken.ThrowIfCancellationRequested();
|
||||||
|
var isAlive = !_disposed &&
|
||||||
|
_started &&
|
||||||
|
_helperProcess is { HasExited: false } &&
|
||||||
|
_commandPipe is { IsConnected: true } &&
|
||||||
|
_outputPipe is { IsConnected: true };
|
||||||
|
return ValueTask.FromResult(isAlive);
|
||||||
|
}
|
||||||
|
|
||||||
public async Task WriteInputAsync(string input, CancellationToken cancellationToken)
|
public async Task WriteInputAsync(string input, CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
if (_commandWriter is null)
|
if (_commandWriter is null)
|
||||||
|
|||||||
@ -6,6 +6,8 @@ internal interface IConPtySession : IAsyncDisposable
|
|||||||
|
|
||||||
Task StartAsync(CancellationToken cancellationToken);
|
Task StartAsync(CancellationToken cancellationToken);
|
||||||
|
|
||||||
|
ValueTask<bool> IsAliveAsync(CancellationToken cancellationToken);
|
||||||
|
|
||||||
Task WriteInputAsync(string input, CancellationToken cancellationToken);
|
Task WriteInputAsync(string input, CancellationToken cancellationToken);
|
||||||
|
|
||||||
Task ResizeAsync(int columns, int rows, CancellationToken cancellationToken);
|
Task ResizeAsync(int columns, int rows, CancellationToken cancellationToken);
|
||||||
|
|||||||
@ -24,9 +24,18 @@ internal sealed class PowerShellSessionHost : ISessionHost, IAsyncDisposable
|
|||||||
ArgumentException.ThrowIfNullOrWhiteSpace(sessionId);
|
ArgumentException.ThrowIfNullOrWhiteSpace(sessionId);
|
||||||
ConPtyInterop.EnsureSupported();
|
ConPtyInterop.EnsureSupported();
|
||||||
|
|
||||||
if (_sessions.ContainsKey(sessionId))
|
if (_sessions.TryGetValue(sessionId, out var existingSession))
|
||||||
{
|
{
|
||||||
return;
|
if (await existingSession.IsAliveAsync(cancellationToken).ConfigureAwait(false))
|
||||||
|
{
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (_sessions.TryRemove(new KeyValuePair<string, IConPtySession>(sessionId, existingSession)))
|
||||||
|
{
|
||||||
|
existingSession.OutputReceived -= HandleSessionOutput;
|
||||||
|
await existingSession.DisposeAsync().ConfigureAwait(false);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!_sessionRegistry.TryGet(sessionId, out var record) || record is null)
|
if (!_sessionRegistry.TryGet(sessionId, out var record) || record is null)
|
||||||
|
|||||||
@ -56,6 +56,25 @@ public class PowerShellSessionHostTests
|
|||||||
Assert.True(history.HasMoreAbove);
|
Assert.True(history.HasMoreAbove);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task StartAsync_Recreates_Unhealthy_Existing_Session()
|
||||||
|
{
|
||||||
|
var factory = new FakeConPtySessionFactory();
|
||||||
|
using var harness = HostHarness.Create(factory);
|
||||||
|
await using var host = harness.Host;
|
||||||
|
var session = harness.Registry.Create("alpha", DateTimeOffset.UtcNow);
|
||||||
|
|
||||||
|
await host.StartAsync(session.SessionId, CancellationToken.None);
|
||||||
|
var firstSession = factory.CreatedSessions.Single();
|
||||||
|
firstSession.IsAlive = false;
|
||||||
|
|
||||||
|
await host.StartAsync(session.SessionId, CancellationToken.None);
|
||||||
|
|
||||||
|
Assert.Equal(2, factory.CreatedSessions.Count);
|
||||||
|
Assert.True(firstSession.DisposeCount > 0);
|
||||||
|
Assert.True(factory.CreatedSessions.Last().StartCount > 0);
|
||||||
|
}
|
||||||
|
|
||||||
private sealed class HostHarness : IDisposable
|
private sealed class HostHarness : IDisposable
|
||||||
{
|
{
|
||||||
private HostHarness(string dataRoot, SessionRegistry registry, PowerShellSessionHost host)
|
private HostHarness(string dataRoot, SessionRegistry registry, PowerShellSessionHost host)
|
||||||
@ -98,20 +117,29 @@ public class PowerShellSessionHostTests
|
|||||||
|
|
||||||
private sealed class FakeConPtySessionFactory : IConPtySessionFactory
|
private sealed class FakeConPtySessionFactory : IConPtySessionFactory
|
||||||
{
|
{
|
||||||
public FakeConPtySession Session { get; } = new();
|
public List<FakeConPtySession> CreatedSessions { get; } = [];
|
||||||
public string? LastWorkingDirectory { get; private set; }
|
public string? LastWorkingDirectory { get; private set; }
|
||||||
|
|
||||||
|
public FakeConPtySession Session => CreatedSessions.Last();
|
||||||
|
|
||||||
public IConPtySession Create(string sessionId, string? workingDirectory = null)
|
public IConPtySession Create(string sessionId, string? workingDirectory = null)
|
||||||
{
|
{
|
||||||
Session.SessionId = sessionId;
|
var session = new FakeConPtySession
|
||||||
|
{
|
||||||
|
SessionId = sessionId,
|
||||||
|
};
|
||||||
|
CreatedSessions.Add(session);
|
||||||
LastWorkingDirectory = workingDirectory;
|
LastWorkingDirectory = workingDirectory;
|
||||||
return Session;
|
return session;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private sealed class FakeConPtySession : IConPtySession
|
private sealed class FakeConPtySession : IConPtySession
|
||||||
{
|
{
|
||||||
public string SessionId { get; set; } = string.Empty;
|
public string SessionId { get; set; } = string.Empty;
|
||||||
|
public bool IsAlive { get; set; } = true;
|
||||||
|
public int StartCount { get; private set; }
|
||||||
|
public int DisposeCount { get; private set; }
|
||||||
|
|
||||||
public event EventHandler<TerminalOutputEventArgs>? OutputReceived;
|
public event EventHandler<TerminalOutputEventArgs>? OutputReceived;
|
||||||
|
|
||||||
@ -119,9 +147,15 @@ public class PowerShellSessionHostTests
|
|||||||
|
|
||||||
public Task StartAsync(CancellationToken cancellationToken)
|
public Task StartAsync(CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
|
StartCount += 1;
|
||||||
return Task.CompletedTask;
|
return Task.CompletedTask;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public ValueTask<bool> IsAliveAsync(CancellationToken cancellationToken)
|
||||||
|
{
|
||||||
|
return ValueTask.FromResult(IsAlive);
|
||||||
|
}
|
||||||
|
|
||||||
public Task WriteInputAsync(string input, CancellationToken cancellationToken)
|
public Task WriteInputAsync(string input, CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
return Task.CompletedTask;
|
return Task.CompletedTask;
|
||||||
@ -140,6 +174,7 @@ public class PowerShellSessionHostTests
|
|||||||
|
|
||||||
public ValueTask DisposeAsync()
|
public ValueTask DisposeAsync()
|
||||||
{
|
{
|
||||||
|
DisposeCount += 1;
|
||||||
return ValueTask.CompletedTask;
|
return ValueTask.CompletedTask;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user