From 4d9ceefee2918a3dd1c9d47134bad5cf7fea4084 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 6 Aug 2026 13:43:12 +0200 Subject: [PATCH] docs(explore-notes): document the ConPTY env race and open-path dedupe fixes --- docs/explore-notes/conpty-sessions.md | 31 ++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/docs/explore-notes/conpty-sessions.md b/docs/explore-notes/conpty-sessions.md index a6370d06..afde9c1f 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 `aac84e4` (2026-08-06). +> Last verified against commit `176ba78` (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. @@ -116,6 +116,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` 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