From 4a28bfe82e0c3c0261a3a5a9e0262a97d34cd8df Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 6 Aug 2026 14:33:24 +0200 Subject: [PATCH] fix(ui): treat an immediate ConPTY exit as a start failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit InteractiveSessionId is persisted before the ConPTY process spawns (1a988ff). If `claude --session-id ` exits immediately after launch (auth/network hiccup, crash before the TUI starts), OnSessionProcessExited set HasExited but never StartError, so ConPtyPaneViewModel.CanRetry (which requires StartError) never offered Retry — every reopen just resumed the same dead session id, permanently poisoning that task's interactive sessions. Now a nonzero exit within 5s of the session becoming "running" is treated as a died-at-startup failure and routed through the same StartError path as a launch-time exception, so the pane shows the error banner + Retry. Retry re-fetches the LaunchSpec via BuildForTaskAsync, which reuses the same persisted InteractiveSessionId — correct for a transient failure (fresh process, same id), but does not help a genuinely dead session id. Clearing a dead session id server-side is a separate design question, left out of scope here. --- .../InteractiveTerminalViewModel.cs | 33 ++++++++++-- .../ConPtyPaneViewModelTests.cs | 16 ++++++ .../InteractiveTerminalViewModelTests.cs | 52 +++++++++++++++++++ 3 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs diff --git a/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs index 60813126..94770137 100644 --- a/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs @@ -13,9 +13,18 @@ namespace ClaudeDo.Ui.ViewModels; /// public sealed partial class InteractiveTerminalViewModel : ViewModelBase, IDisposable { + // A process that dies within this window of becoming "running" is treated as a startup + // failure (see OnSessionProcessExited) rather than a normal session end, even though the + // spawn itself succeeded — e.g. `claude --session-id ` exiting immediately on an + // auth/network hiccup. Long enough to clear normal CLI startup, short enough not to + // mistake a real crash-after-use for a start failure. + private static readonly TimeSpan StartupGraceWindow = TimeSpan.FromSeconds(5); + private readonly PtyTerminalSession _session = new(); + private readonly Func _utcNow; private TerminalControl? _control; private TerminalLaunchDescriptor? _pendingDescriptor; + private DateTime? _startedAtUtc; [ObservableProperty] private bool _isRunning; [ObservableProperty] private bool _hasExited; @@ -27,12 +36,23 @@ public sealed partial class InteractiveTerminalViewModel : ViewModelBase, IDispo /// can show a spinner instead of an empty black pane. public bool IsStarting => !IsRunning && !HasExited && StartError is null; - partial void OnIsRunningChanged(bool value) => OnPropertyChanged(nameof(IsStarting)); + partial void OnIsRunningChanged(bool value) + { + OnPropertyChanged(nameof(IsStarting)); + if (value) _startedAtUtc = _utcNow(); + } + partial void OnHasExitedChanged(bool value) => OnPropertyChanged(nameof(IsStarting)); partial void OnStartErrorChanged(string? value) => OnPropertyChanged(nameof(IsStarting)); - public InteractiveTerminalViewModel() + public InteractiveTerminalViewModel() : this(() => DateTime.UtcNow) { + } + + /// Test-only seam for controlling "time since launch" without real delays. + internal InteractiveTerminalViewModel(Func utcNow) + { + _utcNow = utcNow; _session.ProcessExited += OnSessionProcessExited; } @@ -76,11 +96,18 @@ public sealed partial class InteractiveTerminalViewModel : ViewModelBase, IDispo } } - private void OnSessionProcessExited(object? sender, int exitCode) + /// Internal (not private) so tests can drive it directly without a real ConPTY spawn. + internal void OnSessionProcessExited(object? sender, int exitCode) { IsRunning = false; HasExited = true; ExitCode = exitCode; + + // Died at startup: route through the same banner as a launch-time failure instead of + // leaving a dead terminal with no Retry affordance (ConPtyPaneViewModel.CanRetry + // requires StartError). + if (exitCode != 0 && _startedAtUtc is { } startedAt && _utcNow() - startedAt < StartupGraceWindow) + StartError = $"Session exited immediately (code {exitCode})."; } /// Reports a failure that happened before could be called (e.g. the diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs index 4f4cf20c..b679b9d0 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs @@ -134,5 +134,21 @@ public class ConPtyPaneViewModelTests Assert.True(pane.RetryCommand.CanExecute(null)); } + // ── Died-at-startup (fix: an immediate ConPTY exit used to leave StartError null, so Retry + // never became reachable and the pane was stuck reusing the same persisted session id) ── + + [Fact] + public void RetryCommand_Enabled_AfterProcessExitsNonzeroRightAfterLaunch() + { + using var pane = NewTaskPane(NeverCompletes); + SetRunning(pane); + Assert.False(pane.RetryCommand.CanExecute(null)); + + pane.Terminal.OnSessionProcessExited(null, 1); + + Assert.NotNull(pane.Terminal.StartError); + Assert.True(pane.RetryCommand.CanExecute(null)); + } + private static void SetRunning(ConPtyPaneViewModel pane) => pane.Terminal.IsRunning = true; } diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs new file mode 100644 index 00000000..29eecc5c --- /dev/null +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs @@ -0,0 +1,52 @@ +using ClaudeDo.Ui.ViewModels; +using Xunit; + +namespace ClaudeDo.Ui.Tests.ViewModels.MissionControl; + +public class InteractiveTerminalViewModelTests +{ + private static readonly DateTime LaunchedAt = new(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + + // ── Died-at-startup detection (fix: an immediate ConPTY exit left StartError null, so + // ConPtyPaneViewModel.CanRetry — which requires StartError — never offered Retry) ──────── + + [Fact] + public void ProcessExitsNonzero_RightAfterLaunch_SetsStartError() + { + using var terminal = new InteractiveTerminalViewModel(() => LaunchedAt); + terminal.IsRunning = true; // simulates a successful ConPTY spawn + + terminal.OnSessionProcessExited(null, 1); + + Assert.False(terminal.IsRunning); + Assert.True(terminal.HasExited); + Assert.Equal(1, terminal.ExitCode); + Assert.NotNull(terminal.StartError); + } + + [Fact] + public void ProcessExitsNonzero_AfterRunningAWhile_DoesNotSetStartError() + { + var exitedAt = LaunchedAt.AddSeconds(10); + var callCount = 0; + using var terminal = new InteractiveTerminalViewModel(() => callCount++ == 0 ? LaunchedAt : exitedAt); + terminal.IsRunning = true; // records LaunchedAt via the first clock call + + terminal.OnSessionProcessExited(null, 1); // crash after running for a while + + Assert.True(terminal.HasExited); + Assert.Null(terminal.StartError); + } + + [Fact] + public void ProcessExitsZero_RightAfterLaunch_DoesNotSetStartError() + { + using var terminal = new InteractiveTerminalViewModel(() => LaunchedAt); + terminal.IsRunning = true; + + terminal.OnSessionProcessExited(null, 0); // user typed exit / session ended cleanly + + Assert.True(terminal.HasExited); + Assert.Null(terminal.StartError); + } +}