Merge branch 'claudedo/12bae32376504fd28bedb3c7202feba5'

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 `ddf68d6` (2026-08-06).
+> Last verified against commit `8dbdfb3` (2026-08-06).
 > 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.
 
@@ -144,6 +144,35 @@ add/remove/column change; focus-mode tab switches re-present content). Two-part
 (`InteractiveTerminalViewModel.IsStarting`) and in place of the refine button while
 `TaskRowViewModel.IsRefining`.
 
+### ⚠️ Gotcha: env-var launch race across sessions
+
+`PtyTerminalSession.StartAsync` applies `TerminalLaunchDescriptor.Env` via
+`Environment.SetEnvironmentVariable` onto the **whole UI process** (Porta.Pty has no per-launch
+env seam — it always inherits the calling process's environment), then calls
+`TerminalControl.LaunchProcess()`. Two sessions starting back-to-back (e.g. planning sessions for
+two different tasks) could interleave: task B's `SetEnvironmentVariable` calls could land between
+task A's env-set and its `LaunchProcess()` fork, so task A's `claude` process inherits B's env
+(e.g. `CLAUDEDO_PLANNING_TOKEN`) and fails its own MCP auth. Fixed by serializing the
+set-env-then-launch critical section behind a process-wide `static SemaphoreSlim(1,1)` in
+`PtyTerminalSession`. Env leakage onto the whole process *after* a launch has forked remains a
+documented limitation — only the fork-time race is closed.
+
+### ⚠️ Gotcha: open-path dedupe races
+
+`MissionControlViewModel.OpenConPtySessionAsync` / `OpenPlanningConPtySessionAsync` dedupe by
+`TaskId` against `ConPtySessions`, but the check ran before an **awaited** DB title lookup and
+only `AddConPtyPane` registers the pane — two rapid invocations for the same task (e.g. a
+double-click) could both pass the dedupe check before either pane existed, opening two panes.
+`OpenMergeHelperConPtySessionAsync` was worse: it awaits `CreateMergeHelperTaskAsync` (which mints
+a brand-new task id every call) *before* any `TaskId` dedupe is even possible, so a double-trigger
+always minted two host tasks in the DB.
+
+Fixed with synchronous, pre-await claims: `_pendingTaskOpens` (shared by the two `TaskId`-keyed
+open paths) and `_pendingMergeHelperLists` (keyed by `listId`, guarding the whole method since
+there's no `TaskId` yet to dedupe on) are `HashSet<string>` fields checked-and-added at method
+entry, before any `await`, and released in a `finally`. A second overlapping call for the same key
+bails out immediately instead of racing past the collection-based dedupe.
+
 ## Focus / key handling
 
 `InteractiveTerminalView` lives in `MissionControlWindow`, so the `FocusClearing` Escape handler

src/ClaudeDo.Ui/Services/PtyTerminalSession.cs

@@ -19,6 +19,14 @@ namespace ClaudeDo.Ui.Services;
 /// </summary>
 public sealed class PtyTerminalSession : IDisposable
 {
+    // Guards the set-env + LaunchProcess critical section below: two sessions starting
+    // back-to-back (e.g. planning sessions for two different tasks) could otherwise interleave
+    // their SetEnvironmentVariable calls before either LaunchProcess() forks, so one process
+    // inherits the other's env (e.g. CLAUDEDO_PLANNING_TOKEN, breaking that session's own MCP
+    // auth). Process-wide env leakage AFTER a launch has forked remains a documented limitation
+    // — Porta.Pty has no per-launch env seam, so the vars stay set on the whole UI process.
+    private static readonly SemaphoreSlim s_launchGate = new(1, 1);
+
     private TerminalControl? _control;
     private bool _disposed;
 
@@ -38,14 +46,22 @@ public sealed class PtyTerminalSession : IDisposable
         _control = control;
         control.ProcessExited += OnControlProcessExited;
 
-        foreach (var (key, value) in descriptor.Env)
-            Environment.SetEnvironmentVariable(key, value);
-
         control.Process = descriptor.Exe;
         control.Args = new List<string>(descriptor.Args);
         control.StartingDirectory = descriptor.Cwd;
 
+        await s_launchGate.WaitAsync(ct);
+        try
+        {
+            foreach (var (key, value) in descriptor.Env)
+                Environment.SetEnvironmentVariable(key, value);
+
             await control.LaunchProcess();
+        }
+        finally
+        {
+            s_launchGate.Release();
+        }
 
         // Permanent reparent mode: TerminalView.OnDetachedFromLogicalTree kills the child
         // process unless BeginReparent() suppressed it, and Mission Control detaches pane

src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs

@@ -26,6 +26,13 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
     // review/merge/status machinery.
     public ObservableCollection<ConPtyPaneViewModel> ConPtySessions { get; } = new();
 
+    // Claims a TaskId/listId for the duration of an Open*ConPtySessionAsync call so a second,
+    // overlapping invocation for the same key (e.g. a double-click) bails out instead of racing
+    // past the ConPtySessions dedupe check before the first call has added its pane. Checked and
+    // claimed synchronously at method entry — before any await — and released in a finally.
+    private readonly HashSet<string> _pendingTaskOpens = new();
+    private readonly HashSet<string> _pendingMergeHelperLists = new();
+
     // Mirror of ConPtySessions typed as the pane abstraction so the layout toggle (grid/tabs)
     // binds one contract rather than a ConPTY-specific type.
     public ObservableCollection<IMissionControlPane> Panes { get; } = new();
@@ -145,7 +152,10 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
             FocusedPane = existing;
             return;
         }
+        if (!_pendingTaskOpens.Add(taskId)) return;
 
+        try
+        {
             var title = taskId;
             try
             {
@@ -158,6 +168,11 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
             AddConPtyPane(new ConPtyPaneViewModel(taskId, title,
                 () => DescribeAsync(() => _worker.GetInteractiveLaunchSpecAsync(taskId))));
         }
+        finally
+        {
+            _pendingTaskOpens.Remove(taskId);
+        }
+    }
 
     // Starts (or resumes) a planning session and hosts it as an embedded ConPTY Command Center
     // pane — the ConPTY replacement for the old external wt planning window. Deduped by TaskId
@@ -170,7 +185,10 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
             FocusedPane = existing;
             return;
         }
+        if (!_pendingTaskOpens.Add(taskId)) return;
 
+        try
+        {
             var title = taskId;
             try
             {
@@ -185,6 +203,11 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
                 ? _worker.GetPlanningResumeLaunchSpecAsync(taskId)
                 : _worker.GetPlanningStartLaunchSpecAsync(taskId))));
         }
+        finally
+        {
+            _pendingTaskOpens.Remove(taskId);
+        }
+    }
 
     // Ad-hoc (task-less) ConPTY session in a user-chosen directory. Never deduped — every call
     // opens a fresh pane, unlike the task-based OpenConPtySessionAsync above.
@@ -207,7 +230,13 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
     public async System.Threading.Tasks.Task OpenMergeHelperConPtySessionAsync(string listId, IReadOnlyList<string> taskIds)
     {
         if (taskIds is not { Count: > 0 }) return;
+        // The TaskId dedupe below can't help here — CreateMergeHelperTaskAsync mints a brand-new
+        // task id every call, so a double-trigger for the same list would always mint two host
+        // tasks before either pane exists to dedupe against. Guard the whole method per listId.
+        if (!_pendingMergeHelperLists.Add(listId)) return;
 
+        try
+        {
             var title = Loc.T("missionControl.mergeHelperTitle");
             var listName = listId;
             try
@@ -240,6 +269,11 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable
             AddConPtyPane(new ConPtyPaneViewModel(taskId, title,
                 () => DescribeAsync(() => _worker.GetMergeHelperLaunchSpecAsync(taskIds, listId))));
         }
+        finally
+        {
+            _pendingMergeHelperLists.Remove(listId);
+        }
+    }
 
     // List-handler handoff: the running session called handoff_list_handler at the end of Phase 2.
     // Replaces the Phase 1-2 tile with a fresh one for the SAME handler task id, which carries out

tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs

@@ -47,6 +47,33 @@ public class MissionControlViewModelTests : IDisposable
     private MissionControlViewModel BuildVm(StubWorkerClient worker)
         => new MissionControlViewModel(new TestDbFactory(NewContext), worker, new UsagePillViewModel(worker));
 
+    // Gates the title/list-name DB lookup behind a manually-released TaskCompletionSource so two
+    // overlapping Open*ConPtySessionAsync calls both run their synchronous dedupe-check prefix
+    // to completion before either resumes past the DB await. This reproduces the open-path races
+    // deterministically: a real async gap (e.g. Task.Yield) resumes its continuation on a
+    // thread-pool thread with no synchronization context to serialize it back onto the caller,
+    // which turns the repro into a genuine (flaky) data race instead of the intended
+    // same-thread double-invocation the bug describes.
+    private sealed class GatedDbFactory : IDbContextFactory<ClaudeDoDbContext>
+    {
+        private readonly Func<ClaudeDoDbContext> _create;
+        private readonly TaskCompletionSource _gate = new();
+        public GatedDbFactory(Func<ClaudeDoDbContext> create) => _create = create;
+        public ClaudeDoDbContext CreateDbContext() => _create();
+        public async Task<ClaudeDoDbContext> CreateDbContextAsync(CancellationToken ct = default)
+        {
+            await _gate.Task;
+            return _create();
+        }
+        public void Release() => _gate.TrySetResult();
+    }
+
+    private MissionControlViewModel BuildGatedVm(StubWorkerClient worker, out GatedDbFactory factory)
+    {
+        factory = new GatedDbFactory(NewContext);
+        return new MissionControlViewModel(factory, worker, new UsagePillViewModel(worker));
+    }
+
     // ── acceptance criterion (a): TaskStarted must NOT add a pane ──────────────
 
     [Fact]
@@ -290,6 +317,49 @@ public class MissionControlViewModelTests : IDisposable
         Assert.Single(vm.ConPtySessions);
     }
 
+    [Fact]
+    public async Task OpenConPtySessionAsync_OverlappingCallsSameTask_ProducesSinglePane()
+    {
+        var worker = new FakeWorker();
+        using var vm = BuildGatedVm(worker, out var factory);
+
+        var t1 = vm.OpenConPtySessionAsync("t1");
+        var t2 = vm.OpenConPtySessionAsync("t1");
+        factory.Release();
+        await System.Threading.Tasks.Task.WhenAll(t1, t2);
+
+        Assert.Single(vm.ConPtySessions);
+        Assert.Single(vm.Panes);
+    }
+
+    [Fact]
+    public async Task OpenPlanningConPtySessionAsync_AddsPane_ToConPtySessionsAndPanes()
+    {
+        var worker = new FakeWorker();
+        using var vm = BuildVm(worker);
+
+        await vm.OpenPlanningConPtySessionAsync("t1", resume: false);
+
+        Assert.Single(vm.ConPtySessions);
+        Assert.Equal("t1", vm.ConPtySessions[0].TaskId);
+        Assert.Single(vm.Panes);
+    }
+
+    [Fact]
+    public async Task OpenPlanningConPtySessionAsync_OverlappingCallsSameTask_ProducesSinglePane()
+    {
+        var worker = new FakeWorker();
+        using var vm = BuildGatedVm(worker, out var factory);
+
+        var t1 = vm.OpenPlanningConPtySessionAsync("t1", resume: false);
+        var t2 = vm.OpenPlanningConPtySessionAsync("t1", resume: false);
+        factory.Release();
+        await System.Threading.Tasks.Task.WhenAll(t1, t2);
+
+        Assert.Single(vm.ConPtySessions);
+        Assert.Single(vm.Panes);
+    }
+
     [Fact]
     public async Task OpenConPtySessionAsync_WorkerThrows_RaisesErrorReported_PaneShowsFailure()
     {
@@ -474,6 +544,25 @@ public class MissionControlViewModelTests : IDisposable
         Assert.Same(firstPane, vm.FocusedPane);
     }
 
+    [Fact]
+    public async Task OpenMergeHelperConPtySessionAsync_OverlappingCallsSameList_CreatesOnlyOneHostTask()
+    {
+        var worker = new FixedTaskIdMergeHelperWorker();
+        var factory = new GatedDbFactory(NewContext);
+        using var vm = new MissionControlViewModel(factory, worker, new UsagePillViewModel(worker));
+
+        // Both calls run their synchronous guard-check prefix before either resumes past the
+        // gated DB lookup — call 2 must lose the race and bail out immediately.
+        var t1 = vm.OpenMergeHelperConPtySessionAsync("L1", new[] { "t1" });
+        var t2 = vm.OpenMergeHelperConPtySessionAsync("L1", new[] { "t1" });
+        factory.Release();
+        await System.Threading.Tasks.Task.WhenAll(t1, t2);
+
+        Assert.Equal(1, worker.CreateCallCount);
+        Assert.Single(vm.ConPtySessions);
+        Assert.Single(vm.Panes);
+    }
+
     [Fact]
     public async Task OpenMergeHelperConPtySessionAsync_EmptySelection_NoPane()
     {