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..e672649 --- /dev/null +++ b/tests/CodeShellManager.Tests/SessionOrderTests.cs @@ -0,0 +1,186 @@ +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")); + } + + // 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) => + 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); + } +}