Skip to content

fix(sidebar): sleeping sessions obey the group filter - #142

Merged
AThraen merged 1 commit into
mainfrom
fix/dormant-group-filter
Sep 30, 2026
Merged

AThraen merged 1 commit into
mainfrom
fix/dormant-group-filter

Conversation

@mortenaslo

Copy link
Copy Markdown
Contributor

Note

@AThraen: I have not tested this manually in the running app. Build is clean and the unit tests pass, but nothing covers sidebar rendering, so the manual checks under Test plan are all still open. Could you run through them, or tell me if you'd rather I do it before review?

Summary

Sleeping (dormant) sessions now respect the group filter in the sidebar. They used to be appended unconditionally at the very bottom, whatever the display mode or the active group tab.

Why

The old comment called that "kept reachable", but on a filtered tab it reads as a leak: every sleeping session from every group, listed under a tab that is supposed to show one. A sleeping session stays reachable anyway, one click away on All.

Implementation notes

Placement now follows GroupDisplayMode:

Mode Dormant rows
FilterStrip Still after the live rows, but only those matching ActiveGroupId. All lists every one.
InlineHeaders At the end of their own group section, counted in that header's badge, and collapsed with the group. Ungrouped now also shows when all its members are asleep.
None Unchanged: all at the bottom.
  • AddDormantSidebarItem only registers the row now; it doesn't add it to the sidebar. RebuildSidebarOrder is the only thing that places rows, because where a row goes depends on the display mode and the filter. Every caller must rebuild afterwards. SleepSession and the wake-failure path now do (replacing a bare RefreshTerminalLayout), and the other callers already did. I checked this against main after feat(sessions): restart a session without losing it #140, and all five callers rebuild, the restart fallback included.
  • Dormant rows are ordered by walking SessionManager.Sessions, not the _dormantSidebarItems dictionary, so they follow drag-reorder like live rows instead of insertion order.
  • Two holes a review pass found in the first version, both fixed here:
    • Stale header after deleting a sleeping row. The ✕ removed the Border in place and never rebuilt. Now that inline headers count dormant rows, that left "Work (3)" over two rows, or an empty header. It rebuilds now.
    • A dormant row with an orphaned GroupId rendered nowhere in InlineHeaders mode, and still kept the empty-state placeholder hidden. RemoveGroup clears GroupId, so this can't happen from inside the app, but ImportExportService loads an arbitrary AppState. Such rows are now shown as ungrouped. Live sessions have the same gap and I left it alone: the proper fix is to normalize orphaned ids on load, which is a separate change.

Test plan

  • dotnet build passes with 0 warnings
  • dotnet test tests/CodeShellManager.Tests/ passes 597/597 (none of these cover sidebar rendering)
  • FilterStrip: sleep one session in each of two groups. Each group tab shows only its own sleeping row, Ungrouped shows only ungrouped ones, and All shows every one.
  • FilterStrip: sleep a session while its group's tab is active. The row appears at once; while a different tab is active, it doesn't appear.
  • InlineHeaders: a sleeping row sits at the end of its own group, the header count includes it, and collapsing the group hides it.
  • InlineHeaders: a group whose only members are asleep still shows its header and rows. Same for Ungrouped.
  • InlineHeaders: delete a sleeping row with its ✕. The header count drops, and the header goes away (Ungrouped) or shows 0 (user group) when it was the last one.
  • Wake from a filtered tab, and force a wake failure (e.g. a WSL session whose distro is gone). The row goes back into the right place.
  • Drag-reorder live sessions, then sleep them. Sleeping rows keep that order.
  • Restart path from feat(sessions): restart a session without losing it #140: a restart that fails falls back to a dormant row in the right group.
  • --clean run: no regressions in the empty-state placeholder.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX

Dormant rows were appended unconditionally at the very bottom of the sidebar,
whatever the display mode or active group tab. The comment called that "kept
reachable", but on a filtered tab it read as a leak: every sleeping session
from every group, under a tab that was supposed to show one.

Placement now follows the display mode:

- FilterStrip: dormant rows still trail the live ones, but only those matching
  ActiveGroupId. "All" still lists every one, so a sleeping session is never
  more than a click away.
- InlineHeaders: each dormant row sits at the end of its own group section and
  counts toward that header's badge, so it collapses with the group. Ungrouped
  now also appears when its only members are asleep.
- None: unchanged.

Rows are resolved by walking SessionManager.Sessions rather than the
_dormantSidebarItems dictionary, so sleeping rows follow drag-reorder the same
way live ones do instead of insertion order.

AddDormantSidebarItem now only registers the row; RebuildSidebarOrder is the
single placer, because where a row goes is a filter/mode decision.
SleepSession and the wake-failure path call it (in place of a bare
RefreshTerminalLayout); the other callers already did.

Two further holes, found in review:

- Deleting a dormant row removed its Border in place and never rebuilt.
  Harmless while dormant rows sat outside the sections, but inline headers now
  count them, so it left "Work (3)" over two rows, or an empty header when it
  was the section's last occupant. It rebuilds now.
- In InlineHeaders mode a dormant session whose GroupId names a group that no
  longer exists rendered nowhere, while still suppressing EmptyState. That
  cannot arise in-app (RemoveGroup clears GroupId), but ImportExportService
  deserializes an arbitrary AppState and nothing reconciles orphan ids, so it
  now buckets as ungrouped. Live sessions have the same gap; closing it
  properly means normalizing on load, and is left for a separate change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX
@mortenaslo
mortenaslo requested a review from AThraen September 30, 2026 11:02
@AThraen
AThraen merged commit fc5a940 into main Sep 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants