Source
- Mode 2 pre-commit review on the settings-panel layout-preservation fix
- Underlying bug: returning from the settings panel rebuilt the AvalonDock
document pane and discarded any user-built split / multi-pane layout.
Fix removes a stray ActivateGroup() call on return; pure Visibility
toggle now preserves the dock state.
Verdict
Commit approved — 0 must-fix, 0 should-fix, 1 advisory suggestion.
Suggestion
S-1 — Active-group invalidation paths after settings round-trip
File: Project/AgentZeroWpf/UI/APP/MainWindow.xaml.cs:1155-1170
The fix removes ActivateGroup(_activeGroupIndex) from the
settings-close path because settings never changes the active group.
That assumption holds today — the settings panel only manages
CliDefinition rows, not CliGroup rows.
If a future feature lets the settings panel mutate the group list (add /
delete / reorder workspaces) without going through RemoveCliGroup or
AddCliGroup on the main window, returning to the CLI panel could find
_activeGroupIndex pointing at a vanished or stale group. The previous
code happened to mask that by always re-activating, which would crash
on out-of-range or silently re-show the wrong group.
A defensive guard at the top of the return branch would survive that
shift without re-introducing the layout-clobber:
```csharp
if (SettingsPanel.Visibility == Visibility.Visible)
{
// Defensive: settings could in principle have invalidated the active
// group index. Snap to a valid one without rebuilding panes.
if (_activeGroupIndex >= _cliGroups.Count)
_activeGroupIndex = _cliGroups.Count - 1;
SwitchToCliPanel();
return;
}
```
Not blocking — today's code path can't trip this — but worth tracking
so a future settings extension doesn't regress us back to the
layout-rebuild trap.
Closes-when
Source
document pane and discarded any user-built split / multi-pane layout.
Fix removes a stray
ActivateGroup()call on return; pure Visibilitytoggle now preserves the dock state.
Verdict
Commit approved — 0 must-fix, 0 should-fix, 1 advisory suggestion.
Suggestion
S-1 — Active-group invalidation paths after settings round-trip
File:
Project/AgentZeroWpf/UI/APP/MainWindow.xaml.cs:1155-1170The fix removes
ActivateGroup(_activeGroupIndex)from thesettings-close path because settings never changes the active group.
That assumption holds today — the settings panel only manages
CliDefinitionrows, notCliGrouprows.If a future feature lets the settings panel mutate the group list (add /
delete / reorder workspaces) without going through
RemoveCliGrouporAddCliGroupon the main window, returning to the CLI panel could find_activeGroupIndexpointing at a vanished or stale group. The previouscode happened to mask that by always re-activating, which would crash
on out-of-range or silently re-show the wrong group.
A defensive guard at the top of the return branch would survive that
shift without re-introducing the layout-clobber:
```csharp
if (SettingsPanel.Visibility == Visibility.Visible)
{
// Defensive: settings could in principle have invalidated the active
// group index. Snap to a valid one without rebuilding panes.
if (_activeGroupIndex >= _cliGroups.Count)
_activeGroupIndex = _cliGroups.Count - 1;
}
```
Not blocking — today's code path can't trip this — but worth tracking
so a future settings extension doesn't regress us back to the
layout-rebuild trap.
Closes-when
_activeGroupIndexvalidated on settings → CLI return, OR settingspanel scope explicitly documented as "definitions only, never groups"