From 7379c1638a308b46adf5060d74e61aa22b36e358 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morten=20Aslo-=C3=98stergaard?= Date: Thu, 29 Oct 2026 15:05:47 +0100 Subject: [PATCH 1/2] fix(sidebar): drag-and-drop lands where you drop, and shows where first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported as "the session doesn't land where I want it, and it's hard to know where it actually lands", most noticeably with groups and a PS session next to a Claude one. A faithful simulation of the old code reproduced it in 7 of 7 scenarios. There was not one cause but five, and one more mechanism that undid even correct drops. The drop was resolved to an INDEX, computed in one list and applied to two others that hold different sessions: 1. The index counted the visible rows, so a "⎇ repo (N)" worktree cluster header shifted it by one. 2. In FilterStrip mode it was an index into the FILTERED rows, applied to SessionManager's full list (every group): dropping into the middle of a group put the session at the top. 3. In inline-headers mode it was a _vm.Sessions index (live only), applied to SessionManager (live + dormant): a sleeping session above the target moved the drop. 4. The same index moved both lists, so the pane grid and the sidebar went out of step. 5. Remove-then-insert at the pre-removal index made every downward move overshoot by one. A drop now resolves to an anchor ("before session X" / "after session Y"), which is the same slot in every list. The logic is in a new WPF-free SidebarDropResolver, fed the rows as actually rendered. SessionManager.MoveSessionRelative finds the anchor after removing the moved session, and the index-based MoveSession is gone from both SessionManager and MainViewModel so the pattern can't come back. The grid is re-aligned to the sidebar order (SessionOrder.AlignTo) instead of moved by the same index, and RegisterSession inserts a woken or restarted session by relative position for the same reason. Then the part that made "PS and Claude" worse: "Open PowerShell here" creates a session in the same repo, so it counts as a worktree sibling, and ApplyClusteredOrder re-sorted EVERY sibling on any Add/Remove and whenever a repo root resolved, i.e. on every restore. A PS session dragged away from its Claude session snapped back the next time anything opened or closed, and the saved order was rewritten on every launch. It also moved SessionManager by a _vm.Sessions index (cause 3 again). It is replaced by PlaceNewSessionsInClusters: only sessions launched with restoring: false are placed, once, when their root resolves, and a drag removes a session from that set. New PS sessions still appear beside their Claude session; after that, the user's order wins. Behaviour change: siblings the user has separated are no longer pulled back together. "Hard to know where it lands": while dragging, a blue insertion line (Views/InsertionLineAdorner) shows the slot. It is computed from the same resolved target the drop applies, so it cannot disagree with the drop. It hides over group headers, which highlight themselves instead. Dropping on an inline group header now lands the session at the top of that group, rather than wherever its old global position fell among the new group's members. Tests: SidebarDropResolverTests and SessionOrderTests, plus SidebarReorderScenarioTests, which runs scenarios 1-6 end-to-end through the real SessionManager, the resolver and the grid alignment. Re-introducing the overshoot, or a grid that doesn't follow the sidebar, makes 4 tests fail each. Scenario 7 (restore) is enforced by the restoring: false gate in LaunchSessionAsync, which unit tests can't reach. Not yet tested manually. CLAUDE.md gains a "Sidebar order and drag-and-drop" section recording why no index may cross between the lists. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX --- CLAUDE.md | 43 +++ src/CodeShellManager/MainWindow.xaml.cs | 324 +++++++++--------- .../Services/SessionManager.cs | 18 +- src/CodeShellManager/Services/SessionOrder.cs | 126 +++++++ .../Services/SidebarDropResolver.cs | 108 ++++++ .../ViewModels/MainViewModel.cs | 37 +- .../Views/InsertionLineAdorner.cs | 53 +++ .../SessionOrderTests.cs | 163 +++++++++ .../SidebarDropResolverTests.cs | 138 ++++++++ .../SidebarReorderScenarioTests.cs | 195 +++++++++++ 10 files changed, 1021 insertions(+), 184 deletions(-) create mode 100644 src/CodeShellManager/Services/SessionOrder.cs create mode 100644 src/CodeShellManager/Services/SidebarDropResolver.cs create mode 100644 src/CodeShellManager/Views/InsertionLineAdorner.cs create mode 100644 tests/CodeShellManager.Tests/SessionOrderTests.cs create mode 100644 tests/CodeShellManager.Tests/SidebarDropResolverTests.cs create mode 100644 tests/CodeShellManager.Tests/SidebarReorderScenarioTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 05afdc7..e1cd609 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,6 +69,8 @@ PTY (ConPTY) → PseudoTerminal → TerminalBridge → WebView2 (xterm.js) | `UpdateService` | GitHub Releases version check; caches result for 24h at `%AppData%/CodeShellManager/update-cache.json` | | `ImportExportService` | Read/write a full `AppState` to a JSON file (settings + sessions backup) | | `SessionConfigEditor` | Diffs/applies a `SessionConfigDraft` onto a `ShellSession`; decides whether the change needs a PTY restart | +| `SidebarDropResolver` | WPF-free: maps a drop Y over the rendered sidebar rows to "before / after session X" (+ the group, in inline-headers mode) and the Y of the insertion line. See "Sidebar order and drag-and-drop" | +| `SessionOrder` | WPF-free order helpers: id-relative moves, re-aligning the grid to the sidebar order, and where a new worktree sibling should be placed. See "Sidebar order and drag-and-drop" | | `DbGate` | Serializes every use of the shared `output.db` `SqliteConnection`. That one connection is handed to `SearchService` *and* to every `OutputIndexer`, and it is not thread-safe — concurrent create/dispose corrupts its internal command list. Acquire as `using var _ = await DbGate.AcquireAsync();` at the top of anything touching it. See issue #102 | | `PwshLocator` | Single answer to "pwsh or powershell?", shared by `RunInstance` (run commands) and `PseudoTerminal.BuildCmdLine` (session wrapper) so they can't disagree. Resolves via `where.exe`. An ordinary executable is accepted from metadata alone (no spawn). A **zero-byte reparse point is ambiguous, not bad**: that shape is a Store App Execution Alias, and a *working* Store install of PowerShell 7 looks identical to a stub left by an uninstalled app — so only that case is settled by actually probing execution. Rejecting the shape outright silently downgraded Store-PS7 users to 5.1 on every session launch | | `ToastHelper` | Tray balloon notifications | @@ -614,6 +616,47 @@ Sessions can be put to sleep instead of closed — the PTY is torn down but the - `OnLoaded` partitions saved sessions: dormant ones go through `AddDormantSidebarItem`; live ones through `LaunchSessionAsync`. - The empty-state placeholder hides whenever `_vm.Sessions.Count > 0` **or** `_dormantSidebarItems.Count > 0`. +## Sidebar order and drag-and-drop + +**`SessionManager.Sessions` order is the one source of truth.** It is what the sidebar renders +and what `state.json` persists. `_vm.Sessions` (the pane grid) is *derived* from it, via +`MainViewModel.SyncSessionOrderFromManager` → `SessionOrder.AlignTo`. + +**Never carry an index from one list to another.** The two lists hold different sessions — +the manager's includes dormant ones and every group, the view model's only live ones — and +the visible sidebar rows are a third list again (cluster headers, group headers, dormant rows, +and in FilterStrip mode only one group). Doing exactly that was the "a dragged session +doesn't land where I want it" bug, and it had five separate causes, each reproduced by a +simulation of the old code before it was fixed: + +1. The drop index counted the visible rows, so a `⎇` worktree cluster header shifted it. +2. In FilterStrip mode, an index into the *filtered* rows was applied to the unfiltered list. +3. A `_vm.Sessions` index was applied to `SessionManager`, which also holds dormant sessions. +4. One index moved both lists, so the grid and the sidebar diverged. +5. Remove-then-insert at the pre-removal index made every downward move overshoot by one. + +So a drop now resolves to an **anchor id** (`SidebarDropResolver`: before X / after Y), which +means the same slot in every list, and `SessionManager.MoveSessionRelative` finds the anchor +*after* removing the moved session. There is deliberately no index-based move left. The +insertion line drawn while dragging (`Views/InsertionLineAdorner`) is computed from that same +resolved target, so it cannot disagree with where the drop lands. `RegisterSession` (new, +woken or restarted sessions joining the grid) inserts by relative position for the same reason. + +Dropping onto an inline **group header** assigns that group and lands the session at the top +of it, directly under the header. Dormant rows are never drop anchors; they always render +after their section's live rows. + +**Auto-clustering places new sessions once and never re-sorts.** "Open PowerShell here" makes +a ` (PS)` session in the same repo as its Claude session; `PlaceNewSessionsInClusters` +moves it next to its sibling when its repo root resolves. Only sessions launched with +`restoring: false` are eligible (`_clusterOnRepoResolve`), and a drag removes a session from +that set. The previous `ApplyClusteredOrder` re-sorted *every* sibling on any Add/Remove and +on every restore, so a PS session dragged away from its Claude session snapped back the next +time anything opened or closed, and the saved order was rewritten on each launch. + +Covered by `SidebarDropResolverTests`, `SessionOrderTests` and `SidebarReorderScenarioTests` +(the end-to-end scenarios, each of which misplaced the session before the fix). + ## Per-Session Run Commands Each session can have a list of "run commands" — labelled command lines invoked by the toolbar ▶ button, the F5 keybinding, or the sidebar right-click submenu. Runs spawn a **separate headless `PseudoTerminal`** in the session's working folder (or a fresh `ssh` connection for SSH parents); they do **not** type into the parent PTY, so a Claude session is untouched. diff --git a/src/CodeShellManager/MainWindow.xaml.cs b/src/CodeShellManager/MainWindow.xaml.cs index 6d821de..f89f36a 100644 --- a/src/CodeShellManager/MainWindow.xaml.cs +++ b/src/CodeShellManager/MainWindow.xaml.cs @@ -51,6 +51,10 @@ public partial class MainWindow : Window // user sees the full set of icons immediately, each with a "loading" indicator. // Items are removed once LaunchSessionAsync registers the real sidebar item. private readonly Dictionary _launchingSidebarItems = []; + + // Sessions created in this run that still need placing next to their worktree siblings + // once their repo root resolves. See PlaceNewSessionsInClusters. + private readonly HashSet _clusterOnRepoResolve = []; /// /// Per-session references to the run-related controls inside the terminal wrapper. /// Used by RefreshTerminalRunControls() to update the play button / chips strip @@ -1389,6 +1393,11 @@ private async Task LaunchSessionAsync(ShellSession session, bool restoring = fal { Log($"LaunchSession START: cmd='{session.Command}' args='{session.Args}' folder='{session.WorkingFolder}' restoring={restoring}"); + // A session created in this run joins its worktree siblings once its repo root + // resolves. Restored, woken and restarted sessions (restoring: true) keep their saved + // slot — that order is the user's. See PlaceNewSessionsInClusters. + if (!restoring) _clusterOnRepoResolve.Add(session.Id); + if (session.LaunchValidationError is { } validationError) { Log($"LaunchSession REFUSED: {validationError}"); @@ -1946,6 +1955,7 @@ static void UpdateGitText(TextBlock tb, SessionViewModel svm) { dragPending = false; System.Windows.DragDrop.DoDragDrop(container, vm.Id, System.Windows.DragDropEffects.Move); + HideDropIndicator(); // however it ended: dropped, cancelled, or released elsewhere } }; @@ -3084,7 +3094,17 @@ private Border BuildInlineGroupHeader(Models.SessionGroup? group, int count, boo { string sessionId = (string)e.Data.GetData(System.Windows.DataFormats.StringFormat); var targets = _vm.ResolveActionTargets(sessionId); + // Land directly under the header it was dropped on: the top of the group. + // Assigning alone left it wherever its old global position happened to fall + // among the new group's members — usually somewhere in the middle. + string groupKey = group?.Id ?? ""; + var firstMember = _sessionManager.Sessions.FirstOrDefault(s => + !s.IsDormant && s.Id != sessionId && (s.GroupId ?? "") == groupKey); + _clusterOnRepoResolve.Remove(sessionId); + if (firstMember != null) + _vm.MoveSessionRelative(sessionId, firstMember.Id, after: false); _vm.AssignSessionsToGroup(targets, group?.Id); + RebuildSidebarOrder(); e.Handled = true; return; } @@ -3253,70 +3273,54 @@ private void RecomputeWorktreeSiblings() } } - // Auto-cluster: pull every session in a multi-sibling repo next to its anchor so - // siblings always group up, even when added/removed out of order or imported from - // a non-worktree creation path. This runs on Add/Remove/Reset and on RepoRoot - // resolve — but NOT on Move (the CollectionChanged filter skips it) so user - // drag-reorder is preserved. - bool reordered = ApplyClusteredOrder(); + // Place freshly created sessions next to their worktree siblings — once, and only those. + bool reordered = PlaceNewSessionsInClusters(); - if ((anyChanged || reordered) && _vm.Settings.ShowWorktreeClusters) + // A reorder changes the sidebar whether or not cluster headers are shown. + if (reordered || (anyChanged && _vm.Settings.ShowWorktreeClusters)) RebuildSidebarOrder(); if (reordered) _ = _vm.SaveStateAsync(); } /// - /// Reorders (and the underlying SessionManager - /// list) so every session sharing a RepoRoot sits adjacent to its first-seen anchor. - /// First-occurrence order is preserved between clusters and for solo sessions, so a - /// non-worktree session never gets shuffled past unrelated ones. Returns true when - /// at least one Move happened. + /// Places each session created in this run next to its worktree siblings — once, when its + /// repo root first resolves — so "Open PowerShell here" or a new worktree shows up beside + /// the session it belongs with. Returns true when anything moved. + /// + /// It deliberately does NOT re-sort the list. It used to: every Add/Remove anywhere, and + /// every repo root resolving (i.e. every restore), pulled ALL siblings back together. So a + /// PS session dragged away from its Claude session snapped back the next time anything + /// opened or closed, and the saved order was rewritten on every launch — the drag never + /// stuck. It also moved SessionManager by a _vm.Sessions index, which counts no dormant + /// sessions, so it shuffled the sidebar too. Now only sessions launched with + /// restoring: false are ever placed, and a drag removes a session from the set + /// even before its root resolves: the user's order always wins. /// - private bool ApplyClusteredOrder() + private bool PlaceNewSessionsInClusters() { - if (_vm.Sessions.Count < 2) return false; + if (_clusterOnRepoResolve.Count == 0) return false; - // Compute the desired order: stable group-by RepoRoot, anchored at first occurrence. - var desired = new List(_vm.Sessions.Count); - var anchorIdx = new Dictionary(StringComparer.OrdinalIgnoreCase); - static string ClusterKey(SessionViewModel s) => - s.RepoRoot is { Length: > 0 } root ? root : "__solo:" + s.Id; + var liveById = new Dictionary(_vm.Sessions.Count); + foreach (var v in _vm.Sessions) liveById[v.Id] = v; - foreach (var s in _vm.Sessions) + bool moved = false; + foreach (var vm in _vm.Sessions.ToList()) { - string key = ClusterKey(s); - if (!anchorIdx.TryGetValue(key, out _)) - { - anchorIdx[key] = desired.Count; - desired.Add(s); - } - else - { - // Insert immediately after the last existing member of this cluster. - int insertAt = anchorIdx[key]; - for (int i = anchorIdx[key]; i < desired.Count; i++) - { - if (ClusterKey(desired[i]) == key) insertAt = i + 1; - } - desired.Insert(insertAt, s); - } - } + if (!_clusterOnRepoResolve.Contains(vm.Id) || string.IsNullOrEmpty(vm.RepoRoot)) continue; + _clusterOnRepoResolve.Remove(vm.Id); - // Apply minimal Move operations to align current order with desired. - bool moved = false; - for (int i = 0; i < desired.Count; i++) - { - if (_vm.Sessions[i].Id == desired[i].Id) continue; - int j = -1; - for (int k = i + 1; k < _vm.Sessions.Count; k++) - if (_vm.Sessions[k].Id == desired[i].Id) { j = k; break; } - if (j <= i) continue; - // Mirror in the SessionManager model so state.json persists the new order. - _sessionManager.MoveSession(_vm.Sessions[j].Id, i); - _vm.Sessions.Move(j, i); - moved = true; + // Rebuilt per session: an earlier placement in this loop changes the order. + var placements = _sessionManager.Sessions + .Select(s => liveById.TryGetValue(s.Id, out var live) + ? new SessionPlacement(s.Id, s.GroupId ?? "", !s.IsDormant, live.RepoRoot) + : new SessionPlacement(s.Id, s.GroupId ?? "", false, null)) + .ToList(); + if (SessionOrder.ClusterAnchorFor(placements, vm.Id) is { } anchor + && _sessionManager.MoveSessionRelative(vm.Id, anchor, after: true)) + moved = true; } + if (moved) _vm.SyncSessionOrderFromManager(); return moved; } @@ -3594,59 +3598,75 @@ private async Task OpenNewWorktreeDialogAsync(SessionViewModel source) // ── Sidebar drag-and-drop ───────────────────────────────────────────────── + // Where a dragged session lands is decided in ONE place — SidebarDropResolver, fed the + // rows exactly as rendered — and the insertion line (DragOver) and the drop itself both use + // that one answer, so the line shows precisely where the session will go. + // + // It used to be an index, which is what made drops land elsewhere: computed among the + // visible rows (cluster headers, dormant rows and a filtered group included), then applied + // to SessionManager.Sessions (dormant sessions, every group) and _vm.Sessions (neither), + // with a remove-then-insert that overshot every downward move by one. The result is an + // anchor id instead, which means the same slot in every list. + private Views.InsertionLineAdorner? _insertionLine; + // Called once from constructor after SidebarSessionList is accessible. private void SetupSidebarDrop() { SidebarSessionList.AllowDrop = true; - SidebarSessionList.DragOver += (_, e) => - { - bool inlineMode = _vm.Settings.GroupDisplayMode == Models.GroupDisplayMode.InlineHeaders; - bool ok = IsSessionDragPayload(e.Data) - || (inlineMode && IsGroupDragPayload(e.Data)); - e.Effects = ok - ? System.Windows.DragDropEffects.Move - : System.Windows.DragDropEffects.None; - e.Handled = true; - }; - SidebarSessionList.Drop += (_, e) => - { - var mode = _vm.Settings.GroupDisplayMode; - if (IsSessionDragPayload(e.Data)) + // handledEventsToo: an inline group header handles DragOver for its own drop (assign to + // that group, land at its top) and highlights itself; the line must then be hidden + // rather than left behind at its last position. + SidebarSessionList.AddHandler(System.Windows.UIElement.DragOverEvent, + new System.Windows.DragEventHandler((_, e) => { - string draggedId = (string)e.Data.GetData(System.Windows.DataFormats.StringFormat); - var pos = e.GetPosition(SidebarSessionList); + if (e.Handled) { HideDropIndicator(); return; } - // Inline mode: detect which group section the cursor fell in. If different - // from the dragged session's current group, reassign — otherwise the move - // would just "snap back" once RebuildSidebarOrder regrouped by GroupId. - if (mode == Models.GroupDisplayMode.InlineHeaders - && _sessionManager.Groups.Count > 0) + if (IsSessionDragPayload(e.Data)) { - var (targetGroupId, sessionsIndex) = ResolveInlineSessionDropTarget(pos.Y); - string normalizedTarget = targetGroupId ?? ""; - var dragged = _vm.Sessions.FirstOrDefault(s => s.Id == draggedId); - if (dragged != null && (dragged.GroupId ?? "") != normalizedTarget) - { - // Apply the cross-section reassign to the whole selection set when - // the dragged session is part of one — mirrors the header-drop UX. - var targets = _vm.ResolveActionTargets(draggedId); - _vm.AssignSessionsToGroup(targets, targetGroupId); - } - _vm.MoveSession(draggedId, sessionsIndex); - RebuildSidebarOrder(); + if (ResolveSessionDrop(e.GetPosition(SidebarSessionList).Y) is { } target) + ShowDropIndicator(target.IndicatorY); + else + HideDropIndicator(); + e.Effects = System.Windows.DragDropEffects.Move; + e.Handled = true; return; } - int targetIndex = GetSidebarDropIndex(pos); - _vm.MoveSession(draggedId, targetIndex); - RebuildSidebarOrder(); + HideDropIndicator(); + bool inlineMode = _vm.Settings.GroupDisplayMode == Models.GroupDisplayMode.InlineHeaders; + e.Effects = inlineMode && IsGroupDragPayload(e.Data) + ? System.Windows.DragDropEffects.Move + : System.Windows.DragDropEffects.None; + e.Handled = true; + }), handledEventsToo: true); + + // DragLeave also fires when the cursor merely crosses from the panel onto one of its own + // rows, so only hide once it has genuinely left the list. A drag that ends anywhere else + // (Esc, dropped outside) is covered by the drag source hiding it when DoDragDrop returns. + SidebarSessionList.DragLeave += (_, e) => + { + var p = e.GetPosition(SidebarSessionList); + if (p.X < 0 || p.Y < 0 + || p.X >= SidebarSessionList.ActualWidth || p.Y >= SidebarSessionList.ActualHeight) + HideDropIndicator(); + }; + + SidebarSessionList.Drop += (_, e) => + { + HideDropIndicator(); + + if (IsSessionDragPayload(e.Data)) + { + string draggedId = (string)e.Data.GetData(System.Windows.DataFormats.StringFormat); + if (ResolveSessionDrop(e.GetPosition(SidebarSessionList).Y) is { } target) + ApplySessionDrop(draggedId, target); return; } // Inline mode: group reorder drops onto the sidebar resolve to a position // among the visible group headers. The fixed FilterStrip's own drop handler // is in SetupGroupStripDrop — that's the path when the strip is showing. - if (mode == Models.GroupDisplayMode.InlineHeaders + if (_vm.Settings.GroupDisplayMode == Models.GroupDisplayMode.InlineHeaders && IsGroupDragPayload(e.Data)) { string payload = (string)e.Data.GetData(System.Windows.DataFormats.StringFormat); @@ -3657,99 +3677,77 @@ private void SetupSidebarDrop() }; } - private int GetSidebarDropIndex(System.Windows.Point pos) - { - var children = SidebarSessionList.Children.OfType().ToList(); - for (int i = 0; i < children.Count; i++) - { - var itemPos = children[i].TranslatePoint(new System.Windows.Point(0, 0), SidebarSessionList); - double midY = itemPos.Y + children[i].ActualHeight / 2; - if (pos.Y < midY) return i; - } - return children.Count; - } - /// - /// In inline-headers mode, maps a drop Y to (target group, _vm.Sessions index). - /// Walks SidebarSessionList in order: each "groupheader:" item switches the current - /// section; session items inside the section define insertion midpoints. If the drop - /// falls past every section's midline, the session lands at the end of whatever - /// section the cursor was last inside. + /// Measures the sidebar rows as rendered and asks where a + /// session dropped at goes. Rows are measured as layout SLOTS (margins + /// included) so neighbouring rows share one boundary — the insertion line sits exactly in + /// the gap between them. /// - private (string? targetGroupId, int sessionsIndex) ResolveInlineSessionDropTarget(double y) + private SidebarDropTarget? ResolveSessionDrop(double y) { - // Pass 1: build per-section bounds + session lists by walking the visible children. - // A section's vertical bounds are [its-header-bottom .. next-header-top] (or [0..]/[..MaxValue] - // for the Ungrouped implicit section / the final section). - var sections = new List<(string? groupId, double endY, List sessions)>(); - string? currentGroupId = null; - double currentEndY = double.MaxValue; - var currentSessions = new List(); - + var rows = new List(SidebarSessionList.Children.Count); foreach (System.Windows.UIElement child in SidebarSessionList.Children) { - if (child is not Border item) continue; - string? tag = item.Tag as string; - if (tag == null) continue; - var itemPos = item.TranslatePoint(new System.Windows.Point(0, 0), SidebarSessionList); + if (child is not FrameworkElement fe || fe.Visibility != Visibility.Visible) continue; + if (fe.Tag is not string tag) continue; + SidebarRowKind kind; + string? id; if (tag.StartsWith("groupheader:")) { - // Close out the current section at the new header's top. - currentEndY = itemPos.Y; - // Skip empty sections: if no sessions appeared before this header - // (e.g. drop above the first header in a no-ungrouped sidebar), an - // empty entry would let Pass 2 match the section but fall through to - // the "past every session" tail and silently retarget to ungrouped. - if (currentSessions.Count > 0) - sections.Add((currentGroupId, currentEndY, currentSessions)); - // Start a new section. - string id = tag.Substring("groupheader:".Length); - currentGroupId = id == GroupFilter.Ungrouped ? null : id; - currentSessions = new List(); - continue; + kind = SidebarRowKind.GroupHeader; + id = tag.Substring("groupheader:".Length); + if (id == GroupFilter.Ungrouped) id = null; } - if (tag.StartsWith("cluster:") || tag.StartsWith("dormant:")) continue; - currentSessions.Add(item); + else if (tag.StartsWith("cluster:")) { kind = SidebarRowKind.ClusterHeader; id = null; } + else if (tag.StartsWith("dormant:")) { kind = SidebarRowKind.Dormant; id = tag.Substring("dormant:".Length); } + else if (tag.StartsWith("launching:")) { kind = SidebarRowKind.Session; id = tag.Substring("launching:".Length); } + else { kind = SidebarRowKind.Session; id = tag; } // live rows are tagged with the session id + + double top = fe.TranslatePoint(new System.Windows.Point(0, 0), SidebarSessionList).Y - fe.Margin.Top; + rows.Add(new SidebarRow(kind, id, top, fe.ActualHeight + fe.Margin.Top + fe.Margin.Bottom)); } - if (currentSessions.Count > 0) - sections.Add((currentGroupId, double.MaxValue, currentSessions)); + return SidebarDropResolver.Resolve(rows, y); + } - // Pass 2: find the section whose end-Y is past the drop Y, then resolve the - // insertion point within it. - foreach (var sec in sections) - { - if (y >= sec.endY) continue; + private void ApplySessionDrop(string draggedId, SidebarDropTarget target) + { + // A drag is the user saying where this session goes, so it is never auto-clustered + // afterwards — see PlaceNewSessionsInClusters. + _clusterOnRepoResolve.Remove(draggedId); - foreach (var sItem in sec.sessions) - { - var sp = sItem.TranslatePoint(new System.Windows.Point(0, 0), SidebarSessionList); - if (y < sp.Y + sItem.ActualHeight / 2) - { - string? sid = sItem.Tag as string; - if (!string.IsNullOrEmpty(sid)) - { - for (int j = 0; j < _vm.Sessions.Count; j++) - if (_vm.Sessions[j].Id == sid) return (sec.groupId, j); - } - } - } + if (target.BeforeSessionId is { } beforeId) + _vm.MoveSessionRelative(draggedId, beforeId, after: false); + else if (target.AfterSessionId is { } afterId) + _vm.MoveSessionRelative(draggedId, afterId, after: true); - // Past every session in this section — insert at the section's tail, i.e. just - // after the last existing member of this group in _vm.Sessions. - int lastIdx = -1; - for (int j = 0; j < _vm.Sessions.Count; j++) - { - if ((_vm.Sessions[j].GroupId ?? "") == (sec.groupId ?? "")) - lastIdx = j; - } - return (sec.groupId, lastIdx < 0 ? _vm.Sessions.Count : lastIdx + 1); + // Inline headers: the section under the cursor is the group. A multi-selection moves + // group together, mirroring the header drop. + if (target.AssignsGroup) + { + var dragged = _sessionManager.Sessions.FirstOrDefault(s => s.Id == draggedId); + if (dragged != null && (dragged.GroupId ?? "") != (target.GroupId ?? "")) + _vm.AssignSessionsToGroup(_vm.ResolveActionTargets(draggedId), target.GroupId); } + RebuildSidebarOrder(); + } - // Past every section (shouldn't normally happen — last section's endY is MaxValue). - return (currentGroupId, _vm.Sessions.Count); + private void ShowDropIndicator(double y) + { + if (_insertionLine == null) + { + // The sidebar sits in a ScrollViewer, whose content presenter supplies the layer — + // so the line scrolls with the rows and is clipped to the viewport. + var layer = System.Windows.Documents.AdornerLayer.GetAdornerLayer(SidebarSessionList); + if (layer == null) return; + _insertionLine = new Views.InsertionLineAdorner(SidebarSessionList); + layer.Add(_insertionLine); + } + _insertionLine.ShowAt(y); } + private void HideDropIndicator() => _insertionLine?.Hide(); + /// /// In inline-headers mode, maps a Y coordinate within SidebarSessionList to a user-group /// insertion index (0-based within SessionManager.Groups). The implicit Ungrouped header diff --git a/src/CodeShellManager/Services/SessionManager.cs b/src/CodeShellManager/Services/SessionManager.cs index d61e873..3dfabf6 100644 --- a/src/CodeShellManager/Services/SessionManager.cs +++ b/src/CodeShellManager/Services/SessionManager.cs @@ -75,16 +75,18 @@ public void SortSessions(Comparison comparison) SessionsChanged?.Invoke(); } - public void MoveSession(string sessionId, int newIndex) + /// + /// Moves a session to sit immediately before (or after) another, by id. Deliberately no + /// index-based overload: this list holds dormant sessions and every group, so an index + /// computed anywhere else (the visible sidebar rows, MainViewModel.Sessions) means + /// a different slot here — which is how a dragged session used to land elsewhere. + /// Returns true when the order changed. + /// + public bool MoveSessionRelative(string sessionId, string anchorId, bool after) { - var session = _sessions.FirstOrDefault(s => s.Id == sessionId); - if (session == null) return; - int cur = _sessions.IndexOf(session); - newIndex = Math.Clamp(newIndex, 0, _sessions.Count - 1); - if (cur == newIndex) return; - _sessions.RemoveAt(cur); - _sessions.Insert(newIndex, session); + if (!SessionOrder.MoveRelative(_sessions, s => s.Id, sessionId, anchorId, after)) return false; SessionsChanged?.Invoke(); + return true; } public void UpdateStatus(string sessionId, SessionStatus status) diff --git a/src/CodeShellManager/Services/SessionOrder.cs b/src/CodeShellManager/Services/SessionOrder.cs new file mode 100644 index 0000000..2cc478c --- /dev/null +++ b/src/CodeShellManager/Services/SessionOrder.cs @@ -0,0 +1,126 @@ +using System.Collections.ObjectModel; + +namespace CodeShellManager.Services; + +/// +/// Order bookkeeping shared by the sidebar (which renders SessionManager.Sessions) +/// and the pane grid (which renders MainViewModel.Sessions). +/// +/// The two lists hold different things — the manager's includes dormant sessions, the +/// view model's only live ones — so a position in one is not a position in the other. +/// SessionManager order is the single source of truth (it is what state.json +/// persists and what the sidebar shows); the grid is re-aligned to it by +/// rather than moved by the same index. Applying one list's index +/// to the other is exactly what used to make a drag land somewhere else. +/// +/// WPF-free so it is unit-testable. +/// +public static class SessionOrder +{ + /// + /// Moves so it sits immediately before (or after) . + /// The anchor's index is found AFTER removal, so a downward move doesn't overshoot by one. + /// Returns true when the order changed. Unknown ids, and a move relative to itself, are no-ops. + /// + public static bool MoveRelative(IList list, Func key, string id, string anchorId, bool after) + { + if (id == anchorId) return false; + int from = IndexOf(list, key, id); + if (from < 0 || IndexOf(list, key, anchorId) < 0) return false; + + var item = list[from]; + list.RemoveAt(from); + int to = IndexOf(list, key, anchorId) + (after ? 1 : 0); + list.Insert(to, item); + return to != from; + } + + /// + /// Reorders in place, using + /// so bound views see moves rather than a reset, until its items follow the relative order + /// of . Items missing from the reference keep their relative + /// order after the known ones. Returns true when anything moved. + /// + public static bool AlignTo(ObservableCollection list, IReadOnlyList reference, Func key) + { + var rank = new Dictionary(reference.Count); + for (int i = 0; i < reference.Count; i++) rank.TryAdd(reference[i], i); + + // Stable sort: OrderBy preserves original order between equal ranks. + var desired = list + .Select((item, i) => (item, r: rank.TryGetValue(key(item), out int v) ? v : int.MaxValue, i)) + .OrderBy(t => t.r).ThenBy(t => t.i) + .Select(t => t.item) + .ToList(); + + bool moved = false; + for (int i = 0; i < desired.Count; i++) + { + if (EqualityComparer.Default.Equals(list[i], desired[i])) continue; + int j = -1; + for (int k = i + 1; k < list.Count; k++) + if (EqualityComparer.Default.Equals(list[k], desired[i])) { j = k; break; } + if (j < 0) continue; + list.Move(j, i); + moved = true; + } + return moved; + } + + /// + /// Index at which to insert into so it lands in + /// the same relative position it has in : before the first item + /// that comes after it there. Used when a live session (re)joins the grid. + /// + public static int InsertionIndex(IReadOnlyList list, IReadOnlyList reference, Func key, string id) + { + int myRank = -1; + for (int i = 0; i < reference.Count; i++) + if (reference[i] == id) { myRank = i; break; } + if (myRank < 0) return list.Count; + + var rank = new Dictionary(reference.Count); + for (int i = 0; i < reference.Count; i++) rank.TryAdd(reference[i], i); + for (int i = 0; i < list.Count; i++) + if (rank.TryGetValue(key(list[i]), out int r) && r > myRank) return i; + return list.Count; + } + + /// + /// For a NEWLY created session that has just learned its repo root: the session to place it + /// after, so it joins its worktree siblings — or null when there is nothing to do (no live + /// sibling in its group, or it already sits next to one). + /// + /// Only ever called for sessions created in this run. It never moves anything else, and is + /// never consulted for restored sessions: the saved order is the user's, and re-sorting it + /// on every restore is what used to undo a drag. + /// + /// Every session, in sidebar (SessionManager) order. + public static string? ClusterAnchorFor(IReadOnlyList ordered, string id) + { + var me = ordered.FirstOrDefault(s => s.Id == id); + if (me is null || !me.IsLive || string.IsNullOrEmpty(me.RepoRoot)) return null; + + // Only the live rows of its own group are ever displayed next to it. + var peers = ordered.Where(s => s.IsLive && s.GroupId == me.GroupId).ToList(); + bool IsSibling(SessionPlacement s) => + s.Id != id && string.Equals(s.RepoRoot, me.RepoRoot, StringComparison.OrdinalIgnoreCase); + + int at = peers.FindIndex(s => s.Id == id); + if ((at > 0 && IsSibling(peers[at - 1])) || (at + 1 < peers.Count && IsSibling(peers[at + 1]))) + return null; + return peers.LastOrDefault(IsSibling)?.Id; + } + + private static int IndexOf(IList list, Func key, string id) + { + for (int i = 0; i < list.Count; i++) + if (key(list[i]) == id) return i; + return -1; + } +} + +/// What needs to know about a session. +/// Empty for ungrouped. +/// Known only for live sessions; null until git resolves it. +public sealed record SessionPlacement(string Id, string GroupId, bool IsLive, string? RepoRoot); diff --git a/src/CodeShellManager/Services/SidebarDropResolver.cs b/src/CodeShellManager/Services/SidebarDropResolver.cs new file mode 100644 index 0000000..1db290e --- /dev/null +++ b/src/CodeShellManager/Services/SidebarDropResolver.cs @@ -0,0 +1,108 @@ +namespace CodeShellManager.Services; + +/// What a rendered sidebar row is, as far as a session drop is concerned. +public enum SidebarRowKind +{ + /// A live session row, or a launching placeholder. The only valid drop anchor. + Session, + /// A sleeping session. Not draggable and not an anchor: dormant rows always trail + /// the live rows of their section, so a drop among them means "after the live rows". + Dormant, + /// An inline group header. Starts a section; is the + /// group id, or null for the Ungrouped section. + GroupHeader, + /// A "⎇ repo (N)" worktree cluster header. Decoration, not a session. + ClusterHeader, +} + +/// +/// One rendered sidebar row, top to bottom, in the sidebar's own coordinates. +/// / describe the row's layout SLOT — margins included — +/// so adjacent slots touch, and the boundary between two rows is a single Y. +/// +public readonly record struct SidebarRow(SidebarRowKind Kind, string? Id, double Top, double Height) +{ + public double Bottom => Top + Height; + public double Mid => Top + Height / 2; +} + +/// +/// Where a dropped session goes. At most one of / +/// is set; both null means "keep its position" (a drop into a +/// group section that shows no live rows). is only meaningful when +/// — i.e. in inline-headers mode, where the section under the +/// cursor decides the group. is where to draw the insertion line, +/// and is computed from the same decision, so the line can never disagree with the drop. +/// +public readonly record struct SidebarDropTarget( + bool AssignsGroup, + string? GroupId, + string? BeforeSessionId, + string? AfterSessionId, + double IndicatorY); + +/// +/// Maps a drop Y on the sidebar to a target expressed as "before / after session X". +/// +/// The previous implementation produced an INDEX, and that was the bug: the index was +/// computed in one list (all visible rows — cluster headers, dormant rows and a filtered +/// group included) and applied to two others (SessionManager.Sessions, which holds +/// dormant sessions and every group; and _vm.Sessions, which holds neither), with a +/// remove-then-insert that overshot every downward move by one. An anchor id means the +/// same thing in every list, so none of that can recur. +/// +/// WPF-free so the rules are unit-testable; MainWindow only measures the rows. +/// +public static class SidebarDropResolver +{ + public static SidebarDropTarget? Resolve(IReadOnlyList rows, double y) + { + if (rows.Count == 0) return null; + + // Inline-headers mode renders group headers; flat modes (None, FilterStrip) don't. + // In a flat mode the whole list is one section and the group never changes. + int start = 0, end = rows.Count; + bool sectioned = false; + int headerIndex = -1; + for (int i = 0; i < rows.Count; i++) + { + if (rows[i].Kind != SidebarRowKind.GroupHeader) continue; + sectioned = true; + if (headerIndex < 0 || rows[i].Top <= y) headerIndex = i; + if (rows[i].Top > y) break; + } + if (sectioned) + { + start = headerIndex + 1; + end = rows.Count; + for (int i = start; i < rows.Count; i++) + if (rows[i].Kind == SidebarRowKind.GroupHeader) { end = i; break; } + } + string? groupId = sectioned ? rows[headerIndex].Id : null; + + // First live row whose midline is below the cursor: insert before it. + int lastSession = -1; + for (int i = start; i < end; i++) + { + if (rows[i].Kind != SidebarRowKind.Session) continue; + if (y < rows[i].Mid) + { + // Draw the line above any cluster header that introduces this row: once the + // dropped session sits before the run, it renders above that header too. + int top = i; + while (top - 1 >= start && rows[top - 1].Kind == SidebarRowKind.ClusterHeader) top--; + return new SidebarDropTarget(sectioned, groupId, rows[i].Id, null, rows[top].Top); + } + lastSession = i; + } + + if (lastSession >= 0) + return new SidebarDropTarget(sectioned, groupId, null, rows[lastSession].Id, rows[lastSession].Bottom); + + // A section with no live rows (empty, collapsed, or only sleeping members): the drop + // assigns the group and leaves the position alone — with no live neighbours there is + // nothing to be positioned relative to. + double indicator = sectioned ? rows[headerIndex].Bottom : rows[0].Top; + return new SidebarDropTarget(sectioned, groupId, null, null, indicator); + } +} diff --git a/src/CodeShellManager/ViewModels/MainViewModel.cs b/src/CodeShellManager/ViewModels/MainViewModel.cs index 18c44fb..3a829d1 100644 --- a/src/CodeShellManager/ViewModels/MainViewModel.cs +++ b/src/CodeShellManager/ViewModels/MainViewModel.cs @@ -363,12 +363,11 @@ void Promote() // Mirror SessionManager order so insert-after-parent (CreateSession with afterSessionId) // also lands the VM at the matching slot — otherwise the model order would be correct - // but the sidebar would still show the new entry at the bottom. - int idx = -1; - for (int i = 0; i < _sessionManager.Sessions.Count; i++) - if (_sessionManager.Sessions[i].Id == vm.Id) { idx = i; break; } - if (idx >= 0 && idx <= Sessions.Count) Sessions.Insert(idx, vm); - else Sessions.Add(vm); + // but the sidebar would still show the new entry at the bottom. Insert by RELATIVE + // position, not by the manager's index: that list also holds dormant sessions, so its + // index is a different slot here (a wake used to land at the wrong grid position). + Sessions.Insert( + SessionOrder.InsertionIndex(Sessions, ManagerOrder(), s => s.Id, vm.Id), vm); ActiveSession = vm; _ = SaveStateAsync(); } @@ -473,17 +472,29 @@ private void SendCommandToActive(string text) public SessionViewModel? GetSlotSession(int slot) => slot < Sessions.Count ? Sessions[slot] : null; - public void MoveSession(string sessionId, int newIndex) + /// + /// Moves a session to sit immediately before (or after) another — the only way the sidebar + /// reorders. The SessionManager list is moved by id, and the grid order is then re-derived + /// from it rather than moved by the same index: the two lists hold different sessions + /// (dormant ones live only in the manager), so an index is a different slot in each. + /// + public void MoveSessionRelative(string sessionId, string anchorId, bool after) { - _sessionManager.MoveSession(sessionId, newIndex); - var vm = Sessions.FirstOrDefault(s => s.Id == sessionId); - if (vm == null) return; - int cur = Sessions.IndexOf(vm); - newIndex = Math.Clamp(newIndex, 0, Sessions.Count - 1); - if (cur != newIndex) Sessions.Move(cur, newIndex); + if (!_sessionManager.MoveSessionRelative(sessionId, anchorId, after)) return; + SyncSessionOrderFromManager(); _ = SaveStateAsync(); } + /// + /// Re-aligns (the pane grid) to the SessionManager's order (the + /// sidebar and state.json). Raises only Move notifications, which MainWindow's + /// CollectionChanged handler deliberately ignores. Returns true when anything moved. + /// + public bool SyncSessionOrderFromManager() => + SessionOrder.AlignTo(Sessions, ManagerOrder(), s => s.Id); + + private List ManagerOrder() => _sessionManager.Sessions.Select(s => s.Id).ToList(); + partial void OnLayoutChanged(LayoutMode value) { if (_suppressLayoutPersist) return; diff --git a/src/CodeShellManager/Views/InsertionLineAdorner.cs b/src/CodeShellManager/Views/InsertionLineAdorner.cs new file mode 100644 index 0000000..bcbf011 --- /dev/null +++ b/src/CodeShellManager/Views/InsertionLineAdorner.cs @@ -0,0 +1,53 @@ +using System.Windows; +using System.Windows.Documents; +using System.Windows.Media; +using Brush = System.Windows.Media.Brush; +using Color = System.Windows.Media.Color; +using Pen = System.Windows.Media.Pen; +using Point = System.Windows.Point; + +namespace CodeShellManager.Views; + +/// +/// The "it will land here" line shown while dragging a session over the sidebar. +/// +/// Drawn as an adorner, not as an element inserted into the list: inserting a row would +/// shift every row below it mid-drag, and the drop is resolved against row positions. +/// The sidebar is plain WPF (no HwndHost), so unlike a terminal pane an adorner +/// can draw over it. +/// +internal sealed class InsertionLineAdorner : Adorner +{ + private static readonly Brush Accent = Freeze(new SolidColorBrush(Color.FromRgb(0x89, 0xb4, 0xfa))); + private static readonly Pen Line = Freeze(new Pen(Accent, 2)); + private double _y = double.NaN; + + public InsertionLineAdorner(UIElement adorned) : base(adorned) + { + IsHitTestVisible = false; // must never intercept the drag it is describing + } + + public void ShowAt(double y) + { + if (Visibility == Visibility.Visible && _y.Equals(y)) return; + _y = y; + Visibility = Visibility.Visible; + InvalidateVisual(); + } + + public void Hide() + { + if (Visibility != Visibility.Collapsed) Visibility = Visibility.Collapsed; + } + + protected override void OnRender(DrawingContext dc) + { + if (double.IsNaN(_y)) return; + double width = AdornedElement.RenderSize.Width; + double y = Math.Round(_y); + dc.DrawEllipse(Accent, null, new Point(5, y), 3.5, 3.5); + dc.DrawLine(Line, new Point(5, y), new Point(Math.Max(5, width - 2), y)); + } + + private static T Freeze(T f) where T : Freezable { f.Freeze(); return f; } +} diff --git a/tests/CodeShellManager.Tests/SessionOrderTests.cs b/tests/CodeShellManager.Tests/SessionOrderTests.cs new file mode 100644 index 0000000..2ad0d9e --- /dev/null +++ b/tests/CodeShellManager.Tests/SessionOrderTests.cs @@ -0,0 +1,163 @@ +using System.Collections.ObjectModel; +using CodeShellManager.Services; +using Xunit; + +namespace CodeShellManager.Tests; + +public class SessionOrderTests +{ + private static List L(params string[] ids) => [.. ids]; + + // ── MoveRelative ────────────────────────────────────────────────────────── + + [Fact] + public void Moving_down_lands_exactly_before_the_anchor() + { + // The old remove-then-insert-at-old-index put 'a' AFTER 'c'. + var list = L("a", "b", "c", "d"); + Assert.True(SessionOrder.MoveRelative(list, x => x, "a", "c", after: false)); + Assert.Equal(L("b", "a", "c", "d"), list); + } + + [Fact] + public void Moving_up_lands_exactly_before_the_anchor() + { + var list = L("a", "b", "c", "d"); + Assert.True(SessionOrder.MoveRelative(list, x => x, "d", "b", after: false)); + Assert.Equal(L("a", "d", "b", "c"), list); + } + + [Fact] + public void After_places_directly_behind_the_anchor() + { + var list = L("a", "b", "c", "d"); + Assert.True(SessionOrder.MoveRelative(list, x => x, "a", "c", after: true)); + Assert.Equal(L("b", "c", "a", "d"), list); + } + + [Theory] + [InlineData("b", "b", false)] // relative to itself + [InlineData("b", "c", false)] // already directly before c + [InlineData("b", "a", true)] // already directly after a + [InlineData("zz", "a", false)] // unknown session + [InlineData("a", "zz", false)] // unknown anchor + public void No_op_moves_report_false_and_change_nothing(string id, string anchor, bool after) + { + var list = L("a", "b", "c"); + Assert.False(SessionOrder.MoveRelative(list, x => x, id, anchor, after)); + Assert.Equal(L("a", "b", "c"), list); + } + + // ── AlignTo ─────────────────────────────────────────────────────────────── + + [Fact] + public void AlignTo_follows_the_reference_ignoring_ids_the_list_does_not_hold() + { + // Reference = SessionManager (with dormant 'zz'); list = the grid (live only). + var grid = new ObservableCollection(["c1", "p1", "c2"]); + Assert.True(SessionOrder.AlignTo(grid, L("zz", "c1", "c2", "p1"), x => x)); + Assert.Equal(["c1", "c2", "p1"], grid); + } + + [Fact] + public void AlignTo_raises_only_moves_never_a_reset() + { + var grid = new ObservableCollection(["b", "a", "c"]); + var actions = new List(); + grid.CollectionChanged += (_, e) => actions.Add(e.Action); + + SessionOrder.AlignTo(grid, L("a", "b", "c"), x => x); + + Assert.NotEmpty(actions); + Assert.All(actions, a => Assert.Equal(System.Collections.Specialized.NotifyCollectionChangedAction.Move, a)); + } + + [Fact] + public void AlignTo_keeps_unknown_items_after_known_ones_in_their_own_order() + { + var grid = new ObservableCollection(["x", "b", "y", "a"]); + SessionOrder.AlignTo(grid, L("a", "b"), x => x); + Assert.Equal(["a", "b", "x", "y"], grid); + } + + [Fact] + public void AlignTo_on_an_aligned_list_reports_false() + { + var grid = new ObservableCollection(["a", "b"]); + Assert.False(SessionOrder.AlignTo(grid, L("zz", "a", "b"), x => x)); + } + + // ── InsertionIndex ──────────────────────────────────────────────────────── + + [Fact] + public void A_woken_session_rejoins_the_grid_at_its_relative_position_not_its_manager_index() + { + // Manager: [zz1, zz2, a, w, b] -> 'w' is at manager index 3, but in the grid + // [a, b] it belongs at 1. The old code inserted at 3, i.e. appended. + var grid = L("a", "b"); + Assert.Equal(1, SessionOrder.InsertionIndex(grid, L("zz1", "zz2", "a", "w", "b"), x => x, "w")); + } + + [Fact] + public void InsertionIndex_appends_when_nothing_follows_or_the_id_is_unknown() + { + var grid = L("a", "b"); + Assert.Equal(2, SessionOrder.InsertionIndex(grid, L("a", "b", "w"), x => x, "w")); + Assert.Equal(2, SessionOrder.InsertionIndex(grid, L("a", "b"), x => x, "nope")); + } + + // ── ClusterAnchorFor ────────────────────────────────────────────────────── + + private static SessionPlacement P(string id, string repo, string group = "", bool live = true) => + new(id, group, live, repo == "" ? null : repo); + + [Fact] + public void A_new_ps_session_is_placed_after_its_claude_sibling() + { + var order = new[] { P("api", @"C:\src\api"), P("web", @"C:\src\web"), P("api (PS)", @"C:\src\api") }; + Assert.Equal("api", SessionOrder.ClusterAnchorFor(order, "api (PS)")); + } + + [Fact] + public void Repo_roots_compare_case_insensitively() + { + var order = new[] { P("api", @"C:\src\API"), P("web", ""), P("api (PS)", @"c:\src\api") }; + Assert.Equal("api", SessionOrder.ClusterAnchorFor(order, "api (PS)")); + } + + [Fact] + public void Already_next_to_a_sibling_needs_no_move() + { + var order = new[] { P("api", "R"), P("api (PS)", "R"), P("web", "W") }; + Assert.Null(SessionOrder.ClusterAnchorFor(order, "api (PS)")); + } + + [Fact] + public void A_dormant_session_between_siblings_does_not_count_as_separating_them() + { + // Dormant rows render after the live ones, so this is visually adjacent already. + var order = new[] { P("api", "R"), P("zz", "", live: false), P("api (PS)", "R") }; + Assert.Null(SessionOrder.ClusterAnchorFor(order, "api (PS)")); + } + + [Fact] + public void A_sibling_in_another_group_is_not_an_anchor() + { + var order = new[] { P("api", "R", group: "claude"), P("web", "W", group: "ps"), P("api (PS)", "R", group: "ps") }; + Assert.Null(SessionOrder.ClusterAnchorFor(order, "api (PS)")); + } + + [Fact] + public void No_repo_root_yet_or_no_siblings_means_no_anchor() + { + Assert.Null(SessionOrder.ClusterAnchorFor([P("api", "R"), P("new", "")], "new")); + Assert.Null(SessionOrder.ClusterAnchorFor([P("api", "R"), P("solo", "S")], "solo")); + } + + [Fact] + public void With_several_siblings_it_goes_after_the_last_one() + { + var order = new[] { P("a1", "R"), P("a2", "R"), P("web", "W"), P("a3", "R") }; + Assert.Equal("a2", SessionOrder.ClusterAnchorFor(order, "a3")); + } +} diff --git a/tests/CodeShellManager.Tests/SidebarDropResolverTests.cs b/tests/CodeShellManager.Tests/SidebarDropResolverTests.cs new file mode 100644 index 0000000..4687184 --- /dev/null +++ b/tests/CodeShellManager.Tests/SidebarDropResolverTests.cs @@ -0,0 +1,138 @@ +using CodeShellManager.Services; +using Xunit; + +namespace CodeShellManager.Tests; + +public class SidebarDropResolverTests +{ + private const double H = 40; + + // Rows stacked top to bottom with no gaps, as layout slots. + private static List Stack(params (SidebarRowKind kind, string? id, double height)[] rows) + { + var list = new List(); + double y = 0; + foreach (var (kind, id, height) in rows) + { + list.Add(new SidebarRow(kind, id, y, height)); + y += height; + } + return list; + } + + private static (SidebarRowKind, string?, double) Live(string id) => (SidebarRowKind.Session, id, H); + private static (SidebarRowKind, string?, double) Asleep(string id) => (SidebarRowKind.Dormant, id, H); + private static (SidebarRowKind, string?, double) Header(string? group) => (SidebarRowKind.GroupHeader, group, 26); + private static (SidebarRowKind, string?, double) Cluster() => (SidebarRowKind.ClusterHeader, null, 22); + + [Fact] + public void Empty_sidebar_has_no_target() + { + Assert.Null(SidebarDropResolver.Resolve([], 10)); + } + + [Fact] + public void Top_half_of_a_row_means_before_it() + { + var rows = Stack(Live("a"), Live("b"), Live("c")); + var t = SidebarDropResolver.Resolve(rows, rows[1].Top + 5)!.Value; + Assert.Equal("b", t.BeforeSessionId); + Assert.Null(t.AfterSessionId); + Assert.Equal(rows[1].Top, t.IndicatorY); + } + + [Fact] + public void Bottom_half_of_a_row_means_before_the_next_one() + { + var rows = Stack(Live("a"), Live("b"), Live("c")); + var t = SidebarDropResolver.Resolve(rows, rows[1].Bottom - 5)!.Value; + Assert.Equal("c", t.BeforeSessionId); + } + + [Fact] + public void Bottom_half_of_the_last_row_means_after_it() + { + var rows = Stack(Live("a"), Live("b")); + var t = SidebarDropResolver.Resolve(rows, rows[1].Bottom - 5)!.Value; + Assert.Null(t.BeforeSessionId); + Assert.Equal("b", t.AfterSessionId); + Assert.Equal(rows[1].Bottom, t.IndicatorY); + } + + [Fact] + public void Flat_mode_never_assigns_a_group() + { + var rows = Stack(Live("a"), Live("b")); + Assert.False(SidebarDropResolver.Resolve(rows, 5)!.Value.AssignsGroup); + } + + [Fact] + public void A_cluster_header_is_not_counted_as_a_row() + { + // ⎇ header above api: aiming at api's top half means "before api", and the line is + // drawn above the header, because that is where the dropped row will render. + var rows = Stack(Live("web"), Cluster(), Live("api"), Live("api (PS)"), Live("docs")); + var t = SidebarDropResolver.Resolve(rows, rows[2].Top + 5)!.Value; + Assert.Equal("api", t.BeforeSessionId); + Assert.Equal(rows[1].Top, t.IndicatorY); + } + + [Fact] + public void Dormant_rows_are_not_anchors_a_drop_among_them_lands_after_the_live_rows() + { + var rows = Stack(Live("a"), Live("b"), Asleep("zz1"), Asleep("zz2")); + var t = SidebarDropResolver.Resolve(rows, rows[2].Top + 5)!.Value; + Assert.Null(t.BeforeSessionId); + Assert.Equal("b", t.AfterSessionId); + } + + [Fact] + public void Inline_the_section_under_the_cursor_decides_the_group() + { + var rows = Stack(Header(null), Live("u1"), Header("claude"), Live("c1"), Live("c2"), Header("ps"), Live("p1")); + var t = SidebarDropResolver.Resolve(rows, rows[4].Top + 5)!.Value; + Assert.True(t.AssignsGroup); + Assert.Equal("claude", t.GroupId); + Assert.Equal("c2", t.BeforeSessionId); + } + + [Fact] + public void Inline_the_ungrouped_section_is_group_null() + { + var rows = Stack(Header(null), Live("u1"), Header("claude"), Live("c1")); + var t = SidebarDropResolver.Resolve(rows, rows[1].Bottom - 5)!.Value; + Assert.True(t.AssignsGroup); + Assert.Null(t.GroupId); + Assert.Equal("u1", t.AfterSessionId); + } + + [Fact] + public void Inline_below_the_last_live_row_of_a_section_stays_in_that_section() + { + // Must not slide into the next group's first slot. + var rows = Stack(Header("claude"), Live("c1"), Live("c2"), Header("ps"), Live("p1")); + var t = SidebarDropResolver.Resolve(rows, rows[2].Bottom - 5)!.Value; + Assert.Equal("claude", t.GroupId); + Assert.Equal("c2", t.AfterSessionId); + } + + [Fact] + public void Inline_a_section_with_no_live_rows_assigns_the_group_and_keeps_the_position() + { + var rows = Stack(Header("claude"), Live("c1"), Header("ps"), Asleep("zz"), Header("misc")); + var t = SidebarDropResolver.Resolve(rows, rows[3].Top + 5)!.Value; + Assert.Equal("ps", t.GroupId); + Assert.Null(t.BeforeSessionId); + Assert.Null(t.AfterSessionId); + Assert.Equal(rows[2].Bottom, t.IndicatorY); + } + + [Fact] + public void Inline_above_the_first_header_belongs_to_the_first_section() + { + var rows = Stack(Header("claude"), Live("c1")); + var t = SidebarDropResolver.Resolve(rows, -3)!.Value; + Assert.Equal("claude", t.GroupId); + Assert.Equal("c1", t.BeforeSessionId); + } +} diff --git a/tests/CodeShellManager.Tests/SidebarReorderScenarioTests.cs b/tests/CodeShellManager.Tests/SidebarReorderScenarioTests.cs new file mode 100644 index 0000000..ce56844 --- /dev/null +++ b/tests/CodeShellManager.Tests/SidebarReorderScenarioTests.cs @@ -0,0 +1,195 @@ +using System.Collections.ObjectModel; +using CodeShellManager.Models; +using CodeShellManager.Services; +using Xunit; + +namespace CodeShellManager.Tests; + +/// +/// End-to-end drag scenarios: a real , rows rendered the way +/// MainWindow.RebuildSidebarOrder renders them, the drop resolved by +/// , and the grid re-aligned with +/// — the same path ApplySessionDrop takes. +/// +/// Scenarios 1-5 each reproduced a misplaced drop against the previous index-based code +/// (simulated faithfully before the fix). The cursor is always aimed at the TOP QUARTER of +/// the row the user means to drop above — no edge-of-midline cases. +/// +public class SidebarReorderScenarioTests +{ + private const double RowH = 40, ClusterH = 22, HeaderH = 26; + + private sealed class World + { + public SessionManager Manager { get; } = new(); + public ObservableCollection Grid { get; } = []; // _vm.Sessions ids + public Dictionary Repo { get; } = []; // SessionViewModel.RepoRoot + public string Mode = "None"; // None | FilterStrip | Inline + public string? ActiveGroup; + public List Groups = []; + + public World Add(string id, string group = "", bool dormant = false, string? repo = null) + { + var s = Manager.CreateSession(id, @"C:\src\" + id, "claude", "", group); + s.Id = id; + s.IsDormant = dormant; + if (!dormant) Grid.Add(id); + if (repo != null) Repo[id] = repo; + return this; + } + + private ShellSession S(string id) => Manager.Sessions.First(s => s.Id == id); + + // Mirrors RebuildSidebarOrder (incl. dormant rows obeying the group filter, #142). + public List Render() + { + var rows = new List<(SidebarRowKind kind, string? id)>(); + + void Live(IEnumerable live) + { + var l = live.ToList(); + for (int i = 0; i < l.Count;) + { + int j = i; + Repo.TryGetValue(l[i].Id, out var root); + while (root != null && j + 1 < l.Count && Repo.GetValueOrDefault(l[j + 1].Id) == root) j++; + if (j > i) rows.Add((SidebarRowKind.ClusterHeader, null)); + for (int k = i; k <= j; k++) rows.Add((SidebarRowKind.Session, l[k].Id)); + i = j + 1; + } + } + + if (Mode == "Inline" && Groups.Count > 0) + { + foreach (var g in new[] { "" }.Concat(Groups)) + { + var members = Manager.Sessions.Where(s => s.GroupId == g).ToList(); + if (g == "" && members.Count == 0) continue; + rows.Add((SidebarRowKind.GroupHeader, g == "" ? null : g)); + Live(members.Where(s => !s.IsDormant)); + rows.AddRange(members.Where(s => s.IsDormant).Select(s => (SidebarRowKind.Dormant, (string?)s.Id))); + } + } + else + { + bool Visible(ShellSession s) => Mode != "FilterStrip" || ActiveGroup == null || s.GroupId == ActiveGroup; + Live(Manager.Sessions.Where(s => !s.IsDormant && Visible(s))); + rows.AddRange(Manager.Sessions.Where(s => s.IsDormant && Visible(s)) + .Select(s => (SidebarRowKind.Dormant, (string?)s.Id))); + } + + var result = new List(); + double y = 0; + foreach (var (kind, id) in rows) + { + double h = kind switch { SidebarRowKind.ClusterHeader => ClusterH, SidebarRowKind.GroupHeader => HeaderH, _ => RowH }; + result.Add(new SidebarRow(kind, id, y, h)); + y += h; + } + return result; + } + + public double AboveRow(string id) + { + var r = Render().First(r => r.Kind == SidebarRowKind.Session && r.Id == id); + return r.Top + r.Height * 0.25; + } + + public double BelowEverything() + { + var last = Render().Last(r => r.Kind == SidebarRowKind.Session); + return last.Bottom - 2; + } + + // Mirrors MainWindow.ApplySessionDrop. + public void Drop(string dragged, double y) + { + var t = SidebarDropResolver.Resolve(Render(), y)!.Value; + if (t.BeforeSessionId is { } b) Manager.MoveSessionRelative(dragged, b, after: false); + else if (t.AfterSessionId is { } a) Manager.MoveSessionRelative(dragged, a, after: true); + if (t.AssignsGroup && S(dragged).GroupId != (t.GroupId ?? "")) + Manager.SetSessionGroup(dragged, t.GroupId); + SessionOrder.AlignTo(Grid, Manager.Sessions.Select(s => s.Id).ToList(), x => x); + } + + public List Sidebar() => + Render().Where(r => r.Kind is SidebarRowKind.Session or SidebarRowKind.Dormant).Select(r => r.Id!).ToList(); + + public List LiveIn(string group) => + Manager.Sessions.Where(s => s.GroupId == group && !s.IsDormant).Select(s => s.Id).ToList(); + } + + [Fact] + public void Scenario1_moving_down_one_slot_does_not_overshoot() + { + var w = new World().Add("a").Add("b").Add("c"); + w.Drop("a", w.AboveRow("c")); + Assert.Equal(["b", "a", "c"], w.Sidebar()); // was [b, c, a] + } + + [Fact] + public void Scenario2_a_worktree_cluster_header_does_not_shift_the_drop() + { + var w = new World().Add("web").Add("api", repo: "R").Add("api (PS)", repo: "R").Add("docs"); + w.Drop("docs", w.AboveRow("api")); + Assert.Equal(["web", "docs", "api", "api (PS)"], w.Sidebar()); // was [web, api, docs, api (PS)] + } + + [Fact] + public void Scenario3_filter_strip_drop_lands_where_aimed_within_the_visible_group() + { + var w = new World { Mode = "FilterStrip", ActiveGroup = "claude", Groups = ["claude", "ps"] } + .Add("p1", "ps").Add("c1", "claude").Add("p2", "ps").Add("c2", "claude").Add("c3", "claude"); + w.Drop("c3", w.AboveRow("c2")); + Assert.Equal(["c1", "c3", "c2"], w.LiveIn("claude")); // was [c3, c1, c2] + Assert.Equal(["p1", "p2"], w.LiveIn("ps")); // the hidden group is untouched + } + + [Fact] + public void Scenario4_inline_cross_group_drop_with_a_sleeping_session_lands_where_aimed() + { + var w = new World { Mode = "Inline", Groups = ["claude", "ps"] } + .Add("zz", "claude", dormant: true).Add("c1", "claude").Add("c2", "claude").Add("p1", "ps"); + w.Drop("p1", w.AboveRow("c2")); + Assert.Equal(["c1", "p1", "c2"], w.LiveIn("claude")); // was [p1, c1, c2] + Assert.Empty(w.LiveIn("ps")); + } + + [Fact] + public void Scenario5_the_pane_grid_follows_the_sidebar_after_a_drop() + { + var w = new World { Mode = "Inline", Groups = ["claude", "ps"] } + .Add("zz", "claude", dormant: true).Add("c1", "claude").Add("c2", "claude").Add("p1", "ps"); + w.Drop("p1", w.AboveRow("c2")); + var liveSidebarOrder = w.Manager.Sessions.Where(s => !s.IsDormant).Select(s => s.Id).ToList(); + Assert.Equal(liveSidebarOrder, w.Grid); // grid was [c1, p1, c2] vs sidebar [p1, c1, c2] + } + + [Fact] + public void Scenario6_a_ps_session_dragged_away_from_its_claude_session_stays_put() + { + // The drop itself was always right here; what undid it was the auto-cluster pass + // re-sorting every sibling on the next Add/Remove. That pass now only ever places a + // session created in this run — the dragged session is removed from that set by + // ApplySessionDrop, and an unrelated new session has nothing to be placed next to. + var w = new World().Add("api", repo: "R").Add("api (PS)", repo: "R").Add("web"); + w.Drop("api (PS)", w.BelowEverything()); + Assert.Equal(["api", "web", "api (PS)"], w.Sidebar()); + + w.Add("new", repo: "N"); + var placements = w.Manager.Sessions + .Select(s => new SessionPlacement(s.Id, s.GroupId, !s.IsDormant, w.Repo.GetValueOrDefault(s.Id))) + .ToList(); + Assert.Null(SessionOrder.ClusterAnchorFor(placements, "new")); + Assert.Equal(["api", "web", "api (PS)", "new"], w.Sidebar()); + } + + [Fact] + public void Scenario6b_dropping_within_the_cluster_keeps_both_siblings_together() + { + var w = new World().Add("web").Add("api", repo: "R").Add("api (PS)", repo: "R"); + w.Drop("api (PS)", w.AboveRow("api")); + Assert.Equal(["web", "api (PS)", "api"], w.Sidebar()); + Assert.Contains(w.Render(), r => r.Kind == SidebarRowKind.ClusterHeader); + } +} From 13a15127393345bc9d7d4f95adc862b3a3f7ff4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Morten=20Aslo-=C3=98stergaard?= Date: Fri, 6 Nov 2026 09:47:01 +0100 Subject: [PATCH 2/2] test(sidebar): carry over #159's restart/wake placement cases PR #159 fixed the same RegisterSession index-space bug with its own SessionOrder.LiveInsertIndex; this branch fixes it with InsertionIndex. Its six placement cases (dormant above, first/last, several dormant, restore in progress, nothing dormant) pass unchanged against InsertionIndex, so #159 can close without losing its coverage. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX --- .../SessionOrderTests.cs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/CodeShellManager.Tests/SessionOrderTests.cs b/tests/CodeShellManager.Tests/SessionOrderTests.cs index 2ad0d9e..e672649 100644 --- a/tests/CodeShellManager.Tests/SessionOrderTests.cs +++ b/tests/CodeShellManager.Tests/SessionOrderTests.cs @@ -106,6 +106,29 @@ public void InsertionIndex_appends_when_nothing_follows_or_the_id_is_unknown() Assert.Equal(2, SessionOrder.InsertionIndex(grid, L("a", "b"), x => x, "nope")); } + // Restart / wake re-insert a VM whose sidebar row never moved. Cases carried over from + // PR #159 (fix/restart-pane-order), which fixed the same RegisterSession bug with its own + // SessionOrder.LiveInsertIndex; that PR was closed in favour of InsertionIndex here. + // Arguments: (live grid, manager order incl. dormant / not-yet-launched, id). + + [Theory] + // Sidebar A, D(dormant), B, C — restarting B while the live list is [A, C]. + [InlineData(new[] { "A", "C" }, new[] { "A", "D", "B", "C" }, "B", 1)] + // First session goes first, with a dormant one above it. + [InlineData(new[] { "B" }, new[] { "D", "A", "B" }, "A", 0)] + // Last session goes last, with dormant ones interleaved. + [InlineData(new[] { "A", "B" }, new[] { "A", "D1", "B", "D2", "C" }, "C", 2)] + // Several dormant sessions above do not accumulate an offset. + [InlineData(new[] { "A", "C" }, new[] { "D1", "D2", "D3", "A", "B", "C" }, "B", 1)] + // Restore loop: A and B are in the model but not yet live when C registers. + [InlineData(new[] { "E" }, new[] { "A", "B", "C", "E" }, "C", 0)] + // Nothing dormant: matches the model index. + [InlineData(new[] { "A", "C" }, new[] { "A", "B", "C" }, "B", 1)] + public void Restart_or_wake_lands_at_its_sidebar_position(string[] live, string[] model, string id, int expected) + { + Assert.Equal(expected, SessionOrder.InsertionIndex(L(live), L(model), x => x, id)); + } + // ── ClusterAnchorFor ────────────────────────────────────────────────────── private static SessionPlacement P(string id, string repo, string group = "", bool live = true) =>