diff --git a/docs/explore-notes/conpty-sessions.md b/docs/explore-notes/conpty-sessions.md index 02db0804..5a0c2821 100644 --- a/docs/explore-notes/conpty-sessions.md +++ b/docs/explore-notes/conpty-sessions.md @@ -1,7 +1,7 @@ # ConPTY interactive sessions & launch specs > **Explore-note — verify before trusting.** Distilled map of a subsystem, not authoritative. -> Last verified against commit `8dbdfb3` (2026-08-06). +> Last verified against commit `79b3580` (2026-08-11). > Drift check: `git log --oneline bdee731..HEAD -- src/ClaudeDo.Worker/Planning src/ClaudeDo.Worker/Hub src/ClaudeDo.Worker/Runner/ClaudeArgsBuilder.cs src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs src/ClaudeDo.Ui/Views/InteractiveTerminalView.axaml` > Stable structure only (no line numbers). See docs/explore-notes/README.md. @@ -115,6 +115,15 @@ share the same prompt/model as their non-final counterpart and differ only in on rendered into the handoff kickoff file (`PromptKind.MergeHelperHandoff`) marking the final round and forbidding further reruns. +Each handoff hands the SAME handler task id to a NEW tile, and `MissionControlViewModel. +OpenMergeHelperHandoffConPtySessionAsync` closes the outgoing phase's pane (`CloseConPtySession`, +which disposes its `PtyTerminalSession` and kills the underlying `claude` process) before opening +the next one — every tile is a live process with the full `mcp__claudedo__*` surface, and the +outgoing session is only ever told to end its turn, never to exit. This keeps the one-pane-per- +`TaskId` invariant that `ConPtySessions` relies on elsewhere (`OpenConPtySessionAsync`'s dedupe, +`OnPaneSubmitForReview`) — both use a plain `FirstOrDefault(s => s.TaskId == taskId)`, not a +"newest wins" lookup. + `MCP_TOOL_TIMEOUT` is 200 s here — `TaskWaitMcpTools` clamps its own timeout to 170 s to stay comfortably under it (see [external-mcp.md](external-mcp.md)). diff --git a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs index acc18da4..4a7da83a 100644 --- a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs @@ -153,9 +153,9 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable public async System.Threading.Tasks.Task OpenConPtySessionAsync(string taskId) { if (string.IsNullOrEmpty(taskId)) return; - // A merge-helper handoff can leave more than one pane for this TaskId (the outgoing phase's - // frozen tile plus the active one) -- LastOrDefault resolves to the newest/active pane. - if (ConPtySessions.LastOrDefault(s => s.TaskId == taskId) is { } existing) + // One pane per TaskId is an invariant -- a merge-helper handoff closes its outgoing pane + // before opening the next phase's (see OpenMergeHelperHandoffConPtySessionAsync). + if (ConPtySessions.FirstOrDefault(s => s.TaskId == taskId) is { } existing) { FocusedPane = existing; return; @@ -284,18 +284,21 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable } // List-handler handoff: the running session called handoff_list_handler at the end of a phase. - // Opens a NEW tile for the SAME handler task id to carry out nextPhase, and deliberately leaves - // the outgoing tile open -- it still shows that phase's finished output, which the wait/merge - // chain can run through several times for one handler task. ConPtySessions can therefore hold - // more than one pane per TaskId from here on; lookups that mean "the current/active session for - // this task" (OpenConPtySessionAsync's dedupe, OnPaneSubmitForReview) use LastOrDefault so they - // resolve to the newest tile rather than a frozen earlier phase. No new task is created here; - // see InteractiveLaunchSpecService.BuildForMergeHelperHandoffAsync. + // Opens a NEW tile for the SAME handler task id to carry out nextPhase. Each tile is a live + // `claude` process with the full mcp__claudedo__* surface, so the outgoing phase's pane is + // closed first -- it was only ever told to end its turn, never to exit, and a multi-phase run + // would otherwise leave one live process per phase behind. This keeps the one-pane-per-TaskId + // invariant intact, so OpenConPtySessionAsync's dedupe and OnPaneSubmitForReview can keep using + // FirstOrDefault. No new task is created here; see + // InteractiveLaunchSpecService.BuildForMergeHelperHandoffAsync. public async System.Threading.Tasks.Task OpenMergeHelperHandoffConPtySessionAsync( string taskId, IReadOnlyList survivingTaskIds, string nextPhase) { if (string.IsNullOrEmpty(taskId) || survivingTaskIds is not { Count: > 0 }) return; + foreach (var stale in ConPtySessions.Where(s => s.TaskId == taskId).ToList()) + CloseConPtySession(stale); + var baseTitle = Loc.T("missionControl.mergeHelperTitle"); var title = baseTitle + Loc.T("missionControl.mergeHelperHandoffTitleSuffix"); try @@ -342,9 +345,9 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable // SubmitTaskForReviewAsync calls, with the loser flashing a spurious footer error. private async void OnPaneSubmitForReview(string taskId) { - // See OpenConPtySessionAsync -- LastOrDefault resolves to the newest/active pane when a - // merge-helper handoff has left an earlier phase's frozen tile in place for this TaskId. - if (ConPtySessions.LastOrDefault(s => s.TaskId == taskId) is not { } pane || pane.IsSubmitPending) + // See OpenConPtySessionAsync -- one pane per TaskId is an invariant, so FirstOrDefault is + // always the current/active pane for this task. + if (ConPtySessions.FirstOrDefault(s => s.TaskId == taskId) is not { } pane || pane.IsSubmitPending) return; pane.IsSubmitPending = true; diff --git a/src/ClaudeDo.Worker/Lifecycle/PromptFileRecovery.cs b/src/ClaudeDo.Worker/Lifecycle/PromptFileRecovery.cs index cffa8452..5dae6917 100644 --- a/src/ClaudeDo.Worker/Lifecycle/PromptFileRecovery.cs +++ b/src/ClaudeDo.Worker/Lifecycle/PromptFileRecovery.cs @@ -24,10 +24,21 @@ public sealed class PromptFileRecovery : IHostedService PromptFiles.ReconcileStaleDefaults(_root); var orphans = PromptFiles.QuarantineOrphans(_root); - if (orphans.Count > 0) - _logger.LogWarning("Prompt file recovery: quarantined {Count} orphaned prompt file(s) into _orphans", orphans.Count); - else + if (orphans.Count == 0) + { _logger.LogInformation("Prompt file recovery: no orphaned prompt files found"); + return Task.CompletedTask; + } + + foreach (var orphan_path in orphans) + { + _logger.LogWarning("Prompt file recovery: quarantined orphaned prompt file {orphan_path}", orphan_path); + if (Path.GetFileNameWithoutExtension(orphan_path) + .StartsWith("merge-helper-execute", StringComparison.OrdinalIgnoreCase)) + _logger.LogWarning( + "Prompt file recovery: {orphan_path} was a customization of the retired merge-helper-execute.md prompt, which was split into merge-helper-wait.md and merge-helper-merge.md; its content was not migrated to either", + orphan_path); + } return Task.CompletedTask; } diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs index 99bc6d6e..fccf5ae8 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs @@ -608,6 +608,53 @@ public class MissionControlViewModelTests : IDisposable Assert.NotNull(error); } + [Fact] + public async Task OpenMergeHelperHandoffConPtySessionAsync_ExistingPaneForTaskId_ClosesOldPane_OpensNewFocusedPane() + { + var worker = new FakeWorker(); + using var vm = BuildVm(worker); + await vm.OpenMergeHelperConPtySessionAsync("L1", new[] { "t1" }); + var outgoingPane = Assert.Single(vm.ConPtySessions); + var taskId = outgoingPane.TaskId!; + + await vm.OpenMergeHelperHandoffConPtySessionAsync(taskId, new[] { "t1" }, "wait"); + + var survivingPane = Assert.Single(vm.ConPtySessions); + Assert.Equal(taskId, survivingPane.TaskId); + Assert.NotSame(outgoingPane, survivingPane); + Assert.Same(survivingPane, vm.FocusedPane); + Assert.Single(vm.Panes); + Assert.Same(survivingPane, vm.Panes[0]); + // The outgoing pane went through CloseConPtySession (not just spliced out of the + // collection) -- it unsubscribed its event handlers, same signal the + // CloseConPtySession_UnsubscribesSubmitForReviewRequested test relies on. + Assert.Equal(0, SubscriberCount(outgoingPane, "SubmitForReviewRequested")); + } + + [Fact] + public async Task OpenMergeHelperHandoffConPtySessionAsync_SubmitForReview_ResolvesToSurvivingPane() + { + var worker = new BlockingSubmitWorker(); + using var vm = BuildVm(worker); + await vm.OpenMergeHelperConPtySessionAsync("L1", new[] { "t1" }); + var outgoingPane = Assert.Single(vm.ConPtySessions); + var taskId = outgoingPane.TaskId!; + + await vm.OpenMergeHelperHandoffConPtySessionAsync(taskId, new[] { "t1" }, "wait"); + var survivingPane = Assert.Single(vm.ConPtySessions); + survivingPane.Terminal.IsRunning = true; // simulate a live hand-driven session + + survivingPane.SubmitForReviewCommand.Execute(null); + + Assert.Equal(1, worker.CallCount); + Assert.True(survivingPane.IsSubmitPending); + + worker.Gate.SetResult(null); + await Task.Delay(20); + + Assert.Empty(vm.ConPtySessions); + } + private sealed class BlockingSubmitWorker : StubWorkerClient { public int CallCount { get; private set; } diff --git a/tests/ClaudeDo.Worker.Tests/Lifecycle/PromptFileRecoveryTests.cs b/tests/ClaudeDo.Worker.Tests/Lifecycle/PromptFileRecoveryTests.cs index 0ebf0b52..636b7376 100644 --- a/tests/ClaudeDo.Worker.Tests/Lifecycle/PromptFileRecoveryTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Lifecycle/PromptFileRecoveryTests.cs @@ -1,12 +1,31 @@ using System.Text.Json; using ClaudeDo.Data; using ClaudeDo.Worker.Lifecycle; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; namespace ClaudeDo.Worker.Tests.Lifecycle; public sealed class PromptFileRecoveryTests { + private sealed class CapturingLogger : ILogger + { + public List Messages { get; } = new(); + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, + Func formatter) + => Messages.Add(formatter(state, exception)); + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + public void Dispose() { } + } + } + [Fact] public async Task StartAsync_ReconcilesStaleDefaultAndQuarantinesOrphan_WithoutTouchingRealEdit() { @@ -46,4 +65,33 @@ public sealed class PromptFileRecoveryTests if (Directory.Exists(root)) Directory.Delete(root, recursive: true); } } + + [Fact] + public async Task StartAsync_QuarantinesOrphanedMergeHelperExecute_WarnNamesFileAndReplacements() + { + var root = Path.Combine(Path.GetTempPath(), $"claudedo_prompts_{Guid.NewGuid():N}"); + try + { + Directory.CreateDirectory(root); + var orphanPath = Path.Combine(root, "merge-helper-execute.md"); + File.WriteAllText(orphanPath, "my old customized triage+wait+merge prompt"); + + var logger = new CapturingLogger(); + var sut = new PromptFileRecovery(logger, root); + + await sut.StartAsync(CancellationToken.None); + + var dest = Path.Combine(root, "_orphans", "merge-helper-execute.md"); + Assert.False(File.Exists(orphanPath)); + Assert.True(File.Exists(dest)); + + Assert.Contains(logger.Messages, m => m.Contains("merge-helper-execute.md") && m.Contains(dest)); + Assert.Contains(logger.Messages, m => + m.Contains("merge-helper-wait.md") && m.Contains("merge-helper-merge.md")); + } + finally + { + if (Directory.Exists(root)) Directory.Delete(root, recursive: true); + } + } }