refactor(agent-config): single AgentConfigEditor for list + task scopes

This commit is contained in:
Mika Kuns
2026-06-23 08:52:49 +02:00
parent 60eb671e8f
commit eb0ddb56d3
14 changed files with 761 additions and 482 deletions

View File

@@ -0,0 +1,131 @@
# Phase 4 — AgentConfigEditor (A2)
Date: 2026-06-23 (picked up after reordering Phase 3 ↔ 4)
Umbrella: `docs/superpowers/plans/2026-06-19-feature-unification-plan.md`
Design: `docs/superpowers/specs/2026-06-19-feature-unification-design.md` (A2)
## Reordering note
Phase 3 (WorktreeActions) was deferred. Its premise — overview rows and the Details
merge section each owning duplicate worktree commands — only half-holds: Details has
no Discard/Keep/ForceRemove, and the two Diff doors open different VMs (`WorktreeModal`
vs `DiffModal`) that only Phase 5 unifies. So Phase 3's clean form depends on Phase 5
(Diff) and a fuller MergeCoordinator (Merge); doing it now would build throwaway
per-surface delegates. **Phase 3 is folded into Phase 5.** Phase 4 (independent, clean
dedup) runs now.
## Scope decision: List + Task only (global left as-is)
The design names three scopes (Global | List | Task). Verified against the tree on
2026-06-23, only **List and Task genuinely duplicate**:
- **List** (`ListSettingsModalViewModel`, "AGENT" section): Model / MaxTurns /
SystemPrompt / AgentFile, each with `InheritedBadge` + `↺` reset; 2-tier
(list→global) badges computed with inline logic (does **not** use the existing
`InheritanceResolver.ResolveList` — which is currently dead code); explicit Save.
- **Task** (`AgentSettingsSectionViewModel`, TaskHeaderBar gear flyout): same four
fields; 3-tier (task→list→global) badges via `InheritanceResolver.Resolve`;
`EffectiveMaxTurns` + `EffectiveSystemPromptHint`; `IsRunning` gate; debounced
auto-save.
**Global** (`GeneralSettingsTabViewModel`, Settings → General) is the root: no
inheritance, no badges, no agent file, no reset — three plain controls (model combo,
max-turns numeric, instructions textbox) plus a global-only PermissionMode, interleaved
with unrelated settings (Language, parallelism, report paths, standup weekday) and
saved batched into one `AppSettingsDto` via the modal Save. Embedding the shared editor
there buys ~3 plain fields at the cost of a degenerate no-badges/no-agent/no-reset mode
plus surgery on the settings save path and a relayout of the most settings-dense view.
**Not worth it — global stays as-is.** (Confirmed with Mika 2026-06-23.)
The real maintenance hazard is the **VM logic** (two copies of badge/reset/inheritance
that already drifted), and the **view** (3 of 4 field blocks are pixel-identical). Both
collapse cleanly for List+Task.
## Target
One `AgentConfigEditorViewModel` + one `AgentConfigEditor` UserControl, instantiated
per surface with a scope. The two host VMs keep only their non-agent concerns and host
the editor as a child.
### `ViewModels/Agent/AgentConfigEditorViewModel.cs` (new)
- `enum AgentConfigScope { List, Task }`
- ctor `(IWorkerClient worker, AgentConfigScope scope)`
- Unified bindable surface (single names both views bind to):
`Model` (string?), `MaxTurns` (decimal?), `SystemPrompt` (string),
`SelectedAgent` (AgentInfo?); `ModelOptions`, `Agents`;
`ModelBadge`/`TurnsBadge`/`AgentBadge`, `ModelInheritedHint`/`TurnsInheritedHint`,
`EffectiveSystemPromptHint`; `EffectiveMaxTurns` (int), `IsRunning`/`IsEnabled`.
- Reset commands: `ResetModel`, `ResetTurns`, `ResetAgent`, `ResetAll`.
- Badges via `InheritanceResolver`: scope==Task → `Resolve(own, list, global)`;
scope==List → `ResolveList(own, global)` (adopts the dead method). One `BadgeFor`
helper covers both (List scope never yields the `List` source).
- Load: `LoadForListAsync(listId)` and `LoadForTaskAsync(TaskEntity entity)` — both
pull agents + app-settings (global defaults); Task also pulls the list tier +
`EffectiveSystemPromptHint`. Localizer-change re-badges (port the `Loc.LanguageChanged`
handler + `IDisposable`).
- Save: `SaveAsync()` is scope-aware — List builds `UpdateListConfigDto`
`UpdateListConfigAsync`; Task builds `UpdateTaskAgentSettingsDto`
`UpdateTaskAgentSettingsAsync`. Task scope also auto-saves debounced (300ms) on field
changes; List does not (the modal Save button calls `SaveAsync`). `SaveAsync` is
directly callable (tests bypass the debounce).
- Task-only `Clear()` + `TaskId`.
### `Views/Controls/AgentConfigEditor.axaml` (+ .axaml.cs) (new)
- `x:DataType` = `AgentConfigEditorViewModel`; host sets `DataContext="{Binding Agent}"`.
- The four field blocks (model/turns/systemprompt/agent) with `InheritedBadge` + `↺`
reset, lifted verbatim from the existing two views (they already match). Agent combo
shows Name + Description (both scopes; harmless for task). `EffectiveSystemPromptHint`
line gated on non-empty (hides for List).
- `StyledProperty<bool> ShowAgentBrowse` (default false). True → render the Browse
button + path line; the browse file-picker code-behind lives here (moved from
`ListSettingsModalView`).
- Shared localization namespace `settings.agentEditor.*` (model/maxTurns/systemPrompt/
agentFile/promptPrepended). Reset tooltip reuses `settings.inherit.resetToInherited`.
### Re-point hosts
- `ListSettingsModalViewModel`: drop the agent fields/badges/resets/option-lists; add
`public AgentConfigEditorViewModel Agent { get; }` (scope=List). `LoadAsync`
`Agent.LoadForListAsync(listId)`. `SaveAsync` keeps `UpdateListAsync` (name/dir) and
adds `await Agent.SaveAsync()`. Keep working-dir browse (`BrowseClicked`).
- `ListSettingsModalView.axaml`: replace the AGENT section body with
`<ctl:AgentConfigEditor DataContext="{Binding Agent}" ShowAgentBrowse="True"/>`; the
section-header "Reset agent settings" button binds `Agent.ResetAllCommand`. Remove the
agent browse code-behind (moved into the control).
- `DetailsIslandViewModel`: `AgentSettings` becomes `AgentConfigEditorViewModel`
(scope=Task). Preserve the call sites: ctor, `EffectiveMaxTurns``TurnsText`
PropertyChanged hook, `IsRunning` push, `Dispose`, `Clear`, `TaskId`,
`LoadForTaskAsync(entity, ct)`.
- `TaskHeaderBar.axaml`: replace the flyout field blocks with
`<ctl:AgentConfigEditor DataContext="{Binding AgentSettings}"/>` (ShowAgentBrowse=false).
Keep the gear button + heading.
- Delete `AgentSettingsSectionViewModel.cs`.
## Tests
- New `tests/ClaudeDo.Ui.Tests/ViewModels/AgentConfigEditorViewModelTests.cs`:
- List scope: badges resolve override-vs-global; resets clear; `SaveAsync` builds the
right `UpdateListConfigDto` (via `StubWorkerClient`).
- Task scope: badges resolve override/list/global; `EffectiveMaxTurns`/
`EffectiveSystemPromptHint` from list tier; resets clear; `SaveAsync` builds the right
`UpdateTaskAgentSettingsDto`.
- `InheritanceResolverTests` unchanged (resolver untouched).
- Existing DetailsIsland* tests must stay green (they construct the VM but don't name the
moved members).
## Acceptance
- `dotnet build -c Release` clean for Ui (+ App).
- `Ui.Tests` + `Localization.Tests` green.
- One editor VM + one control drive both List and Task; duplicated field/badge/reset
logic deleted; `ResolveList` now has a real caller.
- Visual gap flagged: open List Settings → Agent, and a task's gear flyout — verify
badges, ↺ resets, reset-all, agent browse (list only), system-prompt hint (task), and
that list Save persists + task auto-saves.
## Commit
`refactor(agent-config): single AgentConfigEditor for list + task scopes`. Stage by
path. Commit this plan with it.