fix(ui): close the outgoing ConPTY pane on merge-helper handoff
Each handoff tile is a live claude process with the full mcp__claudedo__* surface; leaving the outgoing phase's pane open leaked one process per phase. Close it via CloseConPtySession before opening the next phase's tile, restoring the one-pane-per-TaskId invariant so OpenConPtySessionAsync's dedupe and OnPaneSubmitForReview can go back to FirstOrDefault.
This commit is contained in:
@@ -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)).
|
||||
|
||||
|
||||
@@ -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<string> 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;
|
||||
|
||||
@@ -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; }
|
||||
|
||||
Reference in New Issue
Block a user