docs(explore-notes): document the ConPTY env race and open-path dedupe fixes
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 `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<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
|
||||
|
||||
Reference in New Issue
Block a user