fix: address review findings round 1 (PR #819)

[github-actions]

Addresses 9 of 10 expert review findings from round 1 on PR #819.

Findings addressed

#	Severity	Fix
2	🟡 MODERATE	GetFlatSessions() now respects Organization.SortMode via ApplySort()
3	🟡 MODERATE	Dashboard caches flat sessions in _dashboardFlatSessions — no render-path recompute
4	🟡 MODERATE	RebuildRenderedGroups filters codespace sessions from flat list (no duplicates)
5	🟡 MODERATE	GetUnoccupiedWorktrees() excludes group-level WorktreeId/CreatedWorktreeIds
6	🟡 MODERATE	Codespace sessions filtered from flat list — no double-rendering
7	🟡 MODERATE	Removed dead + Group and 🤖 Multi toolbar buttons
8	🟡 MODERATE	JsSwitchToSessionByIndex uses GetFlatSessions() to match dashboard order
9	🟢 MINOR	Removed dead showGroupHeaders variable, simplified codespace conditionals
10	🟢 MINOR	Removed dead OnMove wiring from flat session rendering block

Finding skipped

#	Severity	Reason
1	🔴 CRITICAL	Multi-agent UI removal is intentional per #818 scope (flat sidebar replaces group-based layout)

Test results

✅ All 3661 tests pass

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• 192.0.2.1

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "192.0.2.1"

See Network Configuration for more information.

Generated by Review & Fix · ● 24.2M · ◷

[github-actions]

Design-Level Findings (outside diff hunks)

🟡 MODERATE — Filter chip counts include codespace sessions but filtered list doesn't (1/3 reviewers, verified)

File: SessionSidebar.razor line 1352 (outside diff)

CountByStatus() uses the sessions field (all sessions including codespace), but clicking a filter chip applies ApplyStatusFilter() to _flatSessionList which excludes codespace sessions.

Scenario: A codespace session is processing. Filter bar shows "⚙ Working 1". User clicks it. _flatSessionList is filtered to processing-only — returns 0 items (the processing session is codespace-only and excluded). User sees "No processing sessions" despite the badge saying "1".

Fix: CountByStatus should count from _flatSessionList instead of sessions:

private int CountByStatus(SessionStatusFilter filter)
{
    if (filter == SessionStatusFilter.All) return _flatSessionList.Count;
    return _flatSessionList.Count(s => GetSessionStatus(s) == filter);
}

---

🟡 MODERATE — _dashboardFlatSessions not updated in several mutation paths (2/3 reviewers)

File: Dashboard.razor lines 1829, 1867, 3513, 3524, 3549 (outside diff)

Several code paths update sessions without updating _dashboardFlatSessions:

• /new slash command (line ~1829)
• /rename slash command (line ~1867)
• SetExpandedModel / recreate path (line ~3513)
• ChangeModelAsync success (line ~3524)
• SetReasoningEffort (line ~3549)

These all follow the pattern: sessions = CopilotService.GetAllSessions().ToList(); await InvokeAsync(SafeRefreshAsync); — but SafeRefreshAsync doesn't update _dashboardFlatSessions. The grid renders with stale data until OnStateChanged → RefreshState() runs.

Mitigation: The stale window is brief and self-correcting (next OnStateChanged fixes it). But contrast with HandleComplete (lines 1220-1221) which correctly updates both fields.

Fix: Add _dashboardFlatSessions = CopilotService.GetFlatSessions(); alongside each sessions = ... call, or centralize into a helper method.

---

🟢 MINOR — Dead code: group-creation fields, records, and methods (2/3 reviewers)

Dashboard.razor:

• _expandedWorkerGroups (line 499): HashSet<string>, never read/written after group rendering removed
• isAddingDashGroup / isAddingMultiAgentGroup (lines 503-504): never set to true since UI buttons removed
• CommitDashGroup(), HandleDashGroupKeyDown(), CommitMultiAgentGroup(), HandleMultiAgentGroupKeyDown() (lines 3048-3078): unreachable

SessionSidebar.razor:

• GroupRenderModel record (line 1358) and BuildGroupRenderModel method (line 1425): never called after group rendering replaced with flat list
• isAddingGroup, isAddingMultiAgentGroup, pendingMultiAgentRepo (lines 1325-1326): state variables used in conditionals (lines 344-366, 514-528, 627-752) but never set to true since the "Group" and "Multi-Agent Team" popover buttons were removed

No runtime impact, but the dead state machine logic (~200 lines in sidebar, ~30 lines in dashboard) will confuse future maintainers.

Fix: Remove dead fields, records, methods, and simplify conditionals that reference never-true state variables.

Warning

⚠️ Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

• api.nuget.org
• dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.nuget.org"
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Generated by Expert Code Review · ● 101.1M · ◷

[PureWeen]

Stale — last fix-round PR before push-to-pull-request-branch fix.