From b93b542328927abe1ca42546c93dc0034bd19953 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 17 Jun 2026 08:55:28 -0400 Subject: [PATCH 1/3] fix(tui): apply scope dialog to all selected plugins The "select scope" dialog (Shift-S) operated only on the focused plugin: openScopeDialogForSelected called getSelectedPlugin (singular) and scopeDialogState carried a single pluginID, so confirming the dialog produced a pending operation for just the cursor item even when multiple plugins were bulk-selected. Extend the scope dialog to act on every bulk-selected plugin (falling back to the focused plugin when nothing is multi-selected, matching the existing bulk install/uninstall UX). scopeDialogState now tracks a list of targets, each with its own original scopes, and applyScopeDialogDelta computes the install/uninstall/scope-change delta per target relative to that plugin's current scopes. The dialog title reflects when multiple plugins are selected. Add regression tests for multi-select target loading, focused-item fallback, and per-target delta computation (install, uninstall, mixed scope-change). Closes #39 --- internal/tui/model.go | 60 ++++++++--- internal/tui/model_test.go | 212 +++++++++++++++++++++++++++++++++++++ internal/tui/update.go | 75 ++++++++++--- internal/tui/view.go | 7 +- 4 files changed, 323 insertions(+), 31 deletions(-) diff --git a/internal/tui/model.go b/internal/tui/model.go index c8df266..afcc571 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -286,12 +286,26 @@ type ConfigState struct { scroll int // Scroll position } +// scopeDialogTarget holds one plugin the scope dialog will act on, along with +// the scopes it was installed at before the dialog opened. Each target's delta +// is computed against its own originalScopes so plugins with different starting +// scopes are handled correctly. +type scopeDialogTarget struct { + originalScopes map[claude.Scope]bool // installed scopes before dialog, for this plugin + pluginID string +} + // scopeDialogState holds the state for the multi-scope dialog. +// +// The dialog can act on one or many plugins (when multiple are bulk-selected). +// pluginID/originalScopes mirror the first target and drive checkbox +// initialization and rendering; targets holds every plugin the dialog applies to. type scopeDialogState struct { - originalScopes map[claude.Scope]bool // installed scopes before dialog - pluginID string - cursor int // highlighted row (0-2) - scopes [3]bool // checkbox state: [user, project, local] + originalScopes map[claude.Scope]bool // installed scopes before dialog (first target) + pluginID string // first target's plugin ID (drives rendering) + targets []scopeDialogTarget // all plugins the dialog applies to + cursor int // highlighted row (0-2) + scopes [3]bool // checkbox state: [user, project, local] } // ProgressState holds state for operation progress. @@ -596,30 +610,46 @@ func (m *Model) toggleEnablement() { } } -// openScopeDialog transitions to the scope dialog for the given plugin. +// openScopeDialog transitions to the scope dialog for a single plugin. // preToggle optionally toggles one scope before showing the dialog. func (m *Model) openScopeDialog(pluginID string, installedScopes map[claude.Scope]bool, preToggle *claude.Scope) { + m.openScopeDialogForTargets( + []scopeDialogTarget{{pluginID: pluginID, originalScopes: maps.Clone(installedScopes)}}, + installedScopes, + preToggle, + ) +} + +// openScopeDialogForTargets transitions to the scope dialog for one or more plugins. +// checkboxScopes seeds the checkbox state (typically the focused plugin's scopes); +// each target's pending operation is later computed against its own originalScopes. +// preToggle optionally toggles one scope before showing the dialog. +func (m *Model) openScopeDialogForTargets(targets []scopeDialogTarget, checkboxScopes map[claude.Scope]bool, preToggle *claude.Scope) { m.mode = ModeScopeDialog - m.main.scopeDialog = scopeDialogState{ - pluginID: pluginID, - originalScopes: maps.Clone(installedScopes), + dialog := scopeDialogState{targets: targets} + if len(targets) > 0 { + dialog.pluginID = targets[0].pluginID + dialog.originalScopes = targets[0].originalScopes } - // Initialize checkboxes from current installed scopes (presence, not enabled value) - _, m.main.scopeDialog.scopes[0] = installedScopes[claude.ScopeUser] - _, m.main.scopeDialog.scopes[1] = installedScopes[claude.ScopeProject] - _, m.main.scopeDialog.scopes[2] = installedScopes[claude.ScopeLocal] + + // Initialize checkboxes from the seed scopes (presence, not enabled value). + _, dialog.scopes[0] = checkboxScopes[claude.ScopeUser] + _, dialog.scopes[1] = checkboxScopes[claude.ScopeProject] + _, dialog.scopes[2] = checkboxScopes[claude.ScopeLocal] // Pre-toggle a scope if requested (e.g., pressing 'l' on multi-scope pre-checks local) if preToggle != nil { switch *preToggle { case claude.ScopeUser: - m.main.scopeDialog.scopes[0] = !m.main.scopeDialog.scopes[0] + dialog.scopes[0] = !dialog.scopes[0] case claude.ScopeProject: - m.main.scopeDialog.scopes[1] = !m.main.scopeDialog.scopes[1] + dialog.scopes[1] = !dialog.scopes[1] case claude.ScopeLocal: - m.main.scopeDialog.scopes[2] = !m.main.scopeDialog.scopes[2] + dialog.scopes[2] = !dialog.scopes[2] } } + + m.main.scopeDialog = dialog } // Update implements tea.Model. diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 13dc68c..3a8406c 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "strings" "testing" @@ -2170,6 +2171,217 @@ func TestRenderScopeDialog(t *testing.T) { } } +// testModelMultiPlugin creates a Model with several plugins under one marketplace +// header so bulk selection can be exercised. Each entry maps a plugin name to the +// scopes it is installed at. +func testModelMultiPlugin(plugins map[string][]claude.Scope) (*Model, *mockClient) { + client := &mockClient{} + m := NewModel(client, "/test/project") + + // Deterministic ordering for stable tests. + names := make([]string, 0, len(plugins)) + for name := range plugins { + names = append(names, name) + } + slices.Sort(names) + + states := []PluginState{ + {Name: "marketplace", Marketplace: "marketplace", IsGroupHeader: true}, + } + for _, name := range names { + scopeMap := make(map[claude.Scope]bool, len(plugins[name])) + for _, s := range plugins[name] { + scopeMap[s] = true + } + states = append(states, PluginState{ + ID: name + "@marketplace", + Name: name, + Marketplace: "marketplace", + InstalledScopes: scopeMap, + }) + } + + m.plugins = states + m.selectedIdx = 1 // first non-header plugin + return m, client +} + +// TestOpenScopeDialogForSelectedMultiSelectTargets verifies that pressing S with +// multiple plugins bulk-selected loads all of them as dialog targets, not just the +// focused one. Regression test for issue #39. +func TestOpenScopeDialogForSelectedMultiSelectTargets(t *testing.T) { + m, _ := testModelMultiPlugin(map[string][]claude.Scope{ + "alpha": {claude.ScopeLocal}, + "beta": {claude.ScopeProject}, + "gamma": {claude.ScopeUser}, + }) + // Bulk-select alpha and beta only (gamma is left out). + m.main.bulkSelected["alpha@marketplace"] = true + m.main.bulkSelected["beta@marketplace"] = true + + m.openScopeDialogForSelected() + + if m.mode != ModeScopeDialog { + t.Fatalf("mode = %v, want ModeScopeDialog", m.mode) + } + if len(m.main.scopeDialog.targets) != 2 { + t.Fatalf("targets = %d, want 2 (only bulk-selected plugins)", len(m.main.scopeDialog.targets)) + } + + gotIDs := map[string]bool{} + for _, target := range m.main.scopeDialog.targets { + gotIDs[target.pluginID] = true + } + if !gotIDs["alpha@marketplace"] || !gotIDs["beta@marketplace"] { + t.Errorf("targets = %v, want alpha and beta", gotIDs) + } + if gotIDs["gamma@marketplace"] { + t.Error("gamma should not be a target (not bulk-selected)") + } +} + +// TestOpenScopeDialogForSelectedFallsBackToFocused verifies the single-plugin UX: +// with no bulk selection, S acts only on the focused plugin. +func TestOpenScopeDialogForSelectedFallsBackToFocused(t *testing.T) { + m, _ := testModelMultiPlugin(map[string][]claude.Scope{ + "alpha": {claude.ScopeLocal}, + "beta": {claude.ScopeProject}, + }) + m.selectedIdx = 1 // focus alpha + + m.openScopeDialogForSelected() + + if len(m.main.scopeDialog.targets) != 1 { + t.Fatalf("targets = %d, want 1 (focused plugin only)", len(m.main.scopeDialog.targets)) + } + if m.main.scopeDialog.targets[0].pluginID != "alpha@marketplace" { + t.Errorf("target = %q, want alpha@marketplace", m.main.scopeDialog.targets[0].pluginID) + } + if m.main.scopeDialog.pluginID != "alpha@marketplace" { + t.Errorf("pluginID = %q, want alpha@marketplace", m.main.scopeDialog.pluginID) + } +} + +// TestApplyScopeDialogDeltaMultiSelectInstall verifies that confirming the dialog +// applies the target scope set to ALL selected plugins, each relative to its own +// installed scopes. The checkboxes represent the desired end state for the whole +// selection. Core regression test for issue #39. +func TestApplyScopeDialogDeltaMultiSelectInstall(t *testing.T) { + m, _ := testModelMultiPlugin(map[string][]claude.Scope{ + "alpha": {claude.ScopeLocal}, // already local + "beta": {}, // not installed anywhere + }) + m.main.bulkSelected["alpha@marketplace"] = true + m.main.bulkSelected["beta@marketplace"] = true + m.openScopeDialogForSelected() + + // Desired end state for everyone: Local only (index 2). + m.main.scopeDialog.scopes = [3]bool{false, false, true} + + m.applyScopeDialogDelta() + + // beta: had nothing, wants Local -> OpInstall [Local]. + if op, ok := m.main.pendingOps["beta@marketplace"]; !ok { + t.Error("beta should have a pending op (Local added)") + } else if op.Type != OpInstall || len(op.Scopes) != 1 || op.Scopes[0] != claude.ScopeLocal { + t.Errorf("beta op = %+v, want OpInstall [Local]", op) + } + + // alpha: Local already installed and is the only desired scope -> no change. + if _, ok := m.main.pendingOps["alpha@marketplace"]; ok { + t.Error("alpha should have no pending op (Local already installed, unchanged)") + } +} + +// TestApplyScopeDialogDeltaMultiSelectUninstall verifies unchecking a scope removes +// it from every selected plugin that had it. +func TestApplyScopeDialogDeltaMultiSelectUninstall(t *testing.T) { + m, _ := testModelMultiPlugin(map[string][]claude.Scope{ + "alpha": {claude.ScopeLocal}, + "beta": {claude.ScopeLocal}, + }) + m.main.bulkSelected["alpha@marketplace"] = true + m.main.bulkSelected["beta@marketplace"] = true + m.openScopeDialogForSelected() + + // Focused plugin (alpha) seeds checkboxes with Local checked; uncheck Local. + m.main.scopeDialog.scopes[2] = false + + m.applyScopeDialogDelta() + + for _, id := range []string{"alpha@marketplace", "beta@marketplace"} { + op, ok := m.main.pendingOps[id] + if !ok { + t.Errorf("%s should have a pending uninstall op", id) + continue + } + if op.Type != OpUninstall || len(op.Scopes) != 1 || op.Scopes[0] != claude.ScopeLocal { + t.Errorf("%s op = %+v, want OpUninstall [Local]", id, op) + } + } +} + +// TestApplyScopeDialogDeltaMultiSelectPerTargetDelta verifies each target's delta is +// computed against its own original scopes, producing different operation types for +// plugins that started in different states. +func TestApplyScopeDialogDeltaMultiSelectPerTargetDelta(t *testing.T) { + m, _ := testModelMultiPlugin(map[string][]claude.Scope{ + "alpha": {claude.ScopeProject}, // has Project, not User + "beta": {claude.ScopeUser}, // already has User + }) + m.main.bulkSelected["alpha@marketplace"] = true + m.main.bulkSelected["beta@marketplace"] = true + m.openScopeDialogForSelected() + + // Desired end state: User only (check User, uncheck everything else). + m.main.scopeDialog.scopes = [3]bool{true, false, false} + + m.applyScopeDialogDelta() + + // alpha: had Project, wants User only -> add User, remove Project => OpScopeChange. + alpha, ok := m.main.pendingOps["alpha@marketplace"] + if !ok { + t.Fatal("alpha should have a pending op") + } + if alpha.Type != OpScopeChange { + t.Errorf("alpha op type = %v, want OpScopeChange", alpha.Type) + } + if len(alpha.Scopes) != 1 || alpha.Scopes[0] != claude.ScopeUser { + t.Errorf("alpha install scopes = %v, want [User]", alpha.Scopes) + } + if len(alpha.UninstallScopes) != 1 || alpha.UninstallScopes[0] != claude.ScopeProject { + t.Errorf("alpha uninstall scopes = %v, want [Project]", alpha.UninstallScopes) + } + + // beta: already at User, wants User only -> no change. + if _, ok := m.main.pendingOps["beta@marketplace"]; ok { + t.Error("beta should have no pending op (already at desired scope)") + } +} + +// TestRenderScopeDialogMultiTargetTitle verifies the dialog title reflects a +// multi-plugin selection. +func TestRenderScopeDialogMultiTargetTitle(t *testing.T) { + client := &mockClient{} + m := NewModel(client, "/test/project") + m.width = 100 + m.height = 30 + m.mode = ModeScopeDialog + m.main.scopeDialog = scopeDialogState{ + pluginID: "alpha@marketplace", + targets: []scopeDialogTarget{ + {pluginID: "alpha@marketplace", originalScopes: map[claude.Scope]bool{}}, + {pluginID: "beta@marketplace", originalScopes: map[claude.Scope]bool{}}, + }, + } + + output := m.renderScopeDialog(m.styles) + + if !strings.Contains(output, "2 selected plugins") { + t.Errorf("output should mention multi-plugin selection, got:\n%s", output) + } +} + // TestExecuteOperationInstallWhenNotInSettings tests plugin-scope-mgmt.AC7.1 // When a plugin is not in settings, OpInstall should call InstallPlugin. func TestExecuteOperationInstallWhenNotInSettings(t *testing.T) { diff --git a/internal/tui/update.go b/internal/tui/update.go index 89bd201..89930ac 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -151,13 +151,40 @@ func (m *Model) handleOperationKeys(msg tea.KeyMsg, keys KeyBindings) { } } -// openScopeDialogForSelected opens the scope dialog for the currently selected plugin. +// openScopeDialogForSelected opens the scope dialog for all bulk-selected plugins, +// falling back to the focused plugin when nothing is multi-selected. The focused +// plugin's installed scopes seed the dialog checkboxes; each target's pending +// operation is computed against its own scopes when the dialog is confirmed. func (m *Model) openScopeDialogForSelected() { - plugin := m.getSelectedPlugin() - if plugin == nil || plugin.IsGroupHeader { + plugins := m.getSelectedPlugins() + if len(plugins) == 0 { return } - m.openScopeDialog(plugin.ID, plugin.InstalledScopes, nil) + + targets := make([]scopeDialogTarget, 0, len(plugins)) + for _, plugin := range plugins { + if plugin.IsGroupHeader { + continue + } + targets = append(targets, scopeDialogTarget{ + pluginID: plugin.ID, + originalScopes: maps.Clone(plugin.InstalledScopes), + }) + } + if len(targets) == 0 { + return + } + + // Seed checkboxes from the focused plugin when it is part of the selection, + // otherwise from the first target. This keeps the single-plugin UX unchanged. + checkboxScopes := targets[0].originalScopes + if focused := m.getSelectedPlugin(); focused != nil && !focused.IsGroupHeader { + if _, ok := m.main.bulkSelected[focused.ID]; ok || len(m.main.bulkSelected) == 0 { + checkboxScopes = focused.InstalledScopes + } + } + + m.openScopeDialogForTargets(targets, checkboxScopes, nil) } // scopeDialogScopes maps cursor index to scope. @@ -191,18 +218,36 @@ func (m *Model) updateScopeDialog(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } -// applyScopeDialogDelta computes the difference between original and current checkbox -// state and generates pending operations. +// applyScopeDialogDelta generates pending operations for every plugin the dialog +// targets. The desired scope set (the checkboxes) is applied to each target +// relative to that target's own original scopes, so plugins that started at +// different scopes each get a correct install/uninstall/scope-change operation. func (m *Model) applyScopeDialogDelta() { dialog := &m.main.scopeDialog - original := dialog.originalScopes + + targets := dialog.targets + if len(targets) == 0 { + // Backward-compatible fallback for callers/tests that set pluginID directly. + targets = []scopeDialogTarget{{pluginID: dialog.pluginID, originalScopes: dialog.originalScopes}} + } + + for _, target := range targets { + m.applyScopeDeltaForTarget(target, dialog.scopes) + } +} + +// applyScopeDeltaForTarget computes the install/uninstall delta between a single +// plugin's original scopes and the desired checkbox state, then records the +// resulting pending operation (or clears it when there is no change). +func (m *Model) applyScopeDeltaForTarget(target scopeDialogTarget, desired [3]bool) { + original := target.originalScopes var installScopes []claude.Scope var uninstallScopes []claude.Scope for i, scope := range scopeDialogScopes { _, wasChecked := original[scope] // presence check, not value (disabled-but-present = checked) - isChecked := dialog.scopes[i] + isChecked := desired[i] if !wasChecked && isChecked { installScopes = append(installScopes, scope) @@ -213,7 +258,7 @@ func (m *Model) applyScopeDialogDelta() { // No changes — clear any existing pending op if len(installScopes) == 0 && len(uninstallScopes) == 0 { - m.clearPending(dialog.pluginID) + m.clearPending(target.pluginID) return } @@ -224,16 +269,16 @@ func (m *Model) applyScopeDialogDelta() { switch { case len(uninstallScopes) > 0 && len(installScopes) == 0: // Pure uninstall (partial or full) - m.main.pendingOps[dialog.pluginID] = Operation{ - PluginID: dialog.pluginID, + m.main.pendingOps[target.pluginID] = Operation{ + PluginID: target.pluginID, Scopes: uninstallScopes, OriginalScopes: maps.Clone(original), Type: OpUninstall, } case len(installScopes) > 0 && len(uninstallScopes) == 0: // Pure install (adding scopes) - m.main.pendingOps[dialog.pluginID] = Operation{ - PluginID: dialog.pluginID, + m.main.pendingOps[target.pluginID] = Operation{ + PluginID: target.pluginID, Scopes: installScopes, OriginalScopes: maps.Clone(original), Type: OpInstall, @@ -242,8 +287,8 @@ func (m *Model) applyScopeDialogDelta() { // Mixed: both install and uninstall — use OpScopeChange // This carries both install and uninstall scope lists. // Phase 7 execution handles uninstalls first, then installs. - m.main.pendingOps[dialog.pluginID] = Operation{ - PluginID: dialog.pluginID, + m.main.pendingOps[target.pluginID] = Operation{ + PluginID: target.pluginID, Scopes: installScopes, UninstallScopes: uninstallScopes, OriginalScopes: maps.Clone(original), diff --git a/internal/tui/view.go b/internal/tui/view.go index 910660f..baaa86f 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -784,8 +784,13 @@ var scopeDialogLabels = [3]struct { func (m *Model) renderScopeDialog(styles Styles) string { dialog := &m.main.scopeDialog + title := " Scopes for " + dialog.pluginID + " " + if n := len(dialog.targets); n > 1 { + title = fmt.Sprintf(" Scopes for %d selected plugins ", n) + } + var lines []string - lines = append(lines, styles.Header.Render(" Scopes for "+dialog.pluginID+" ")) + lines = append(lines, styles.Header.Render(title)) lines = append(lines, "") for i := range 3 { From e22374d8e338fd8548247140866aa48d94af3b86 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 17 Jun 2026 09:06:55 -0400 Subject: [PATCH 2/3] refactor(tui): clone scope map defensively and cover fallback path - Clone focused.InstalledScopes when seeding the scope dialog checkboxes, matching the maps.Clone style used for every target's originalScopes and avoiding aliasing live plugin state if the reference is later retained. - Add TestApplyScopeDialogDeltaTargetlessFallback to explicitly exercise the len(targets)==0 backward-compatible fallback in applyScopeDialogDelta, and document the path from the fallback comment. --- internal/tui/model_test.go | 34 ++++++++++++++++++++++++++++++++++ internal/tui/update.go | 7 +++++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 3a8406c..57b5e56 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -2109,6 +2109,40 @@ func TestApplyScopeDialogDeltaNoChange(t *testing.T) { } } +// TestApplyScopeDialogDeltaTargetlessFallback explicitly exercises the +// backward-compatible fallback in applyScopeDialogDelta: when the dialog has no +// targets slice, the delta is computed from the top-level pluginID/originalScopes +// fields instead. Production code always populates targets, so this guards the +// fallback that direct scopeDialogState assignments (e.g. older tests) rely on. +func TestApplyScopeDialogDeltaTargetlessFallback(t *testing.T) { + client := &mockClient{} + m := NewModel(client, "/test/project") + // No targets set — only the legacy top-level fields. + m.main.scopeDialog = scopeDialogState{ + pluginID: "legacy@marketplace", + scopes: [3]bool{true, false, false}, + originalScopes: map[claude.Scope]bool{}, + } + m.main.pendingOps = make(map[string]Operation) + + if len(m.main.scopeDialog.targets) != 0 { + t.Fatalf("precondition failed: targets should be empty, got %d", len(m.main.scopeDialog.targets)) + } + + m.applyScopeDialogDelta() + + op, ok := m.main.pendingOps["legacy@marketplace"] + if !ok { + t.Fatal("expected pending operation derived from the targetless fallback") + } + if op.Type != OpInstall { + t.Errorf("Type = %v, want OpInstall", op.Type) + } + if len(op.Scopes) != 1 || op.Scopes[0] != claude.ScopeUser { + t.Errorf("Scopes = %v, want [ScopeUser]", op.Scopes) + } +} + // TestRenderScopeDialog tests dialog rendering with checkbox display. // Verifies plugin-scope-mgmt.AC6.1 and AC6.4: Dialog renders with scope names and file paths. func TestRenderScopeDialog(t *testing.T) { diff --git a/internal/tui/update.go b/internal/tui/update.go index 89930ac..12d2efc 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -180,7 +180,7 @@ func (m *Model) openScopeDialogForSelected() { checkboxScopes := targets[0].originalScopes if focused := m.getSelectedPlugin(); focused != nil && !focused.IsGroupHeader { if _, ok := m.main.bulkSelected[focused.ID]; ok || len(m.main.bulkSelected) == 0 { - checkboxScopes = focused.InstalledScopes + checkboxScopes = maps.Clone(focused.InstalledScopes) } } @@ -227,7 +227,10 @@ func (m *Model) applyScopeDialogDelta() { targets := dialog.targets if len(targets) == 0 { - // Backward-compatible fallback for callers/tests that set pluginID directly. + // Backward-compatible fallback for callers that set pluginID directly + // without a targets slice. Production code always populates targets via + // openScopeDialogForTargets; this path is covered by + // TestApplyScopeDialogDeltaTargetlessFallback. targets = []scopeDialogTarget{{pluginID: dialog.pluginID, originalScopes: dialog.originalScopes}} } From 73b087255e212ae2254cd1fad1058e272a3956c3 Mon Sep 17 00:00:00 2001 From: piekstra Date: Wed, 17 Jun 2026 09:44:40 -0400 Subject: [PATCH 3/3] refactor(tui): drop redundant scope-dialog originalScopes mirror The scopeDialogState.originalScopes field aliased targets[0].originalScopes, so mutating either backing map silently corrupted the other. The field was read only by a targetless fallback that existed solely for direct struct assignments in tests. Remove the mirror field (and the now-dead fallback), reading original scopes from each target instead, and convert the affected tests to populate targets directly. Also add coverage for openScopeDialogForSelected when the focused plugin is not bulk-selected: targets contain only the bulk selection and the dialog checkboxes seed from the first target's scopes, not the focused plugin's. --- internal/tui/model.go | 14 ++-- internal/tui/model_test.go | 145 ++++++++++++++++++++++--------------- internal/tui/update.go | 11 +-- 3 files changed, 94 insertions(+), 76 deletions(-) diff --git a/internal/tui/model.go b/internal/tui/model.go index afcc571..23534c0 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -298,14 +298,13 @@ type scopeDialogTarget struct { // scopeDialogState holds the state for the multi-scope dialog. // // The dialog can act on one or many plugins (when multiple are bulk-selected). -// pluginID/originalScopes mirror the first target and drive checkbox -// initialization and rendering; targets holds every plugin the dialog applies to. +// pluginID mirrors the first target and drives the single-plugin title; targets +// holds every plugin the dialog applies to, each carrying its own original scopes. type scopeDialogState struct { - originalScopes map[claude.Scope]bool // installed scopes before dialog (first target) - pluginID string // first target's plugin ID (drives rendering) - targets []scopeDialogTarget // all plugins the dialog applies to - cursor int // highlighted row (0-2) - scopes [3]bool // checkbox state: [user, project, local] + pluginID string // first target's plugin ID (drives rendering) + targets []scopeDialogTarget // all plugins the dialog applies to + cursor int // highlighted row (0-2) + scopes [3]bool // checkbox state: [user, project, local] } // ProgressState holds state for operation progress. @@ -629,7 +628,6 @@ func (m *Model) openScopeDialogForTargets(targets []scopeDialogTarget, checkboxS dialog := scopeDialogState{targets: targets} if len(targets) > 0 { dialog.pluginID = targets[0].pluginID - dialog.originalScopes = targets[0].originalScopes } // Initialize checkboxes from the seed scopes (presence, not enabled value). diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 57b5e56..c451fdb 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -1860,9 +1860,11 @@ func TestUpdateScopeDialogUpDown(t *testing.T) { m := NewModel(client, "/test/project") m.mode = ModeScopeDialog m.main.scopeDialog = scopeDialogState{ - pluginID: "test@marketplace", - cursor: 1, - originalScopes: map[claude.Scope]bool{claude.ScopeProject: true}, + pluginID: "test@marketplace", + cursor: 1, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{claude.ScopeProject: true}}, + }, } m.keys = DefaultKeyBindings() @@ -1904,10 +1906,12 @@ func TestUpdateScopeDialogSpaceToggle(t *testing.T) { m := NewModel(client, "/test/project") m.mode = ModeScopeDialog m.main.scopeDialog = scopeDialogState{ - pluginID: "test@marketplace", - cursor: 0, - scopes: [3]bool{false, false, false}, - originalScopes: map[claude.Scope]bool{}, + pluginID: "test@marketplace", + cursor: 0, + scopes: [3]bool{false, false, false}, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{}}, + }, } // Toggle on User scope @@ -1946,9 +1950,11 @@ func TestUpdateScopeDialogEnter(t *testing.T) { m := NewModel(client, "/test/project") m.mode = ModeScopeDialog m.main.scopeDialog = scopeDialogState{ - pluginID: "test@marketplace", - scopes: [3]bool{true, false, false}, - originalScopes: map[claude.Scope]bool{}, + pluginID: "test@marketplace", + scopes: [3]bool{true, false, false}, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{}}, + }, } m.main.pendingOps = make(map[string]Operation) m.keys = DefaultKeyBindings() @@ -1982,9 +1988,11 @@ func TestUpdateScopeDialogEscape(t *testing.T) { m := NewModel(client, "/test/project") m.mode = ModeScopeDialog m.main.scopeDialog = scopeDialogState{ - pluginID: "test@marketplace", - scopes: [3]bool{true, false, false}, - originalScopes: map[claude.Scope]bool{}, + pluginID: "test@marketplace", + scopes: [3]bool{true, false, false}, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{}}, + }, } m.main.pendingOps = make(map[string]Operation) m.keys = DefaultKeyBindings() @@ -2010,9 +2018,11 @@ func TestApplyScopeDialogDeltaCheckInstall(t *testing.T) { client := &mockClient{} m := NewModel(client, "/test/project") m.main.scopeDialog = scopeDialogState{ - pluginID: "test@marketplace", - scopes: [3]bool{true, false, false}, - originalScopes: map[claude.Scope]bool{}, + pluginID: "test@marketplace", + scopes: [3]bool{true, false, false}, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{}}, + }, } m.main.pendingOps = make(map[string]Operation) @@ -2036,9 +2046,11 @@ func TestApplyScopeDialogDeltaCheckUninstall(t *testing.T) { client := &mockClient{} m := NewModel(client, "/test/project") m.main.scopeDialog = scopeDialogState{ - pluginID: "test@marketplace", - scopes: [3]bool{false, false, false}, - originalScopes: map[claude.Scope]bool{claude.ScopeLocal: true}, + pluginID: "test@marketplace", + scopes: [3]bool{false, false, false}, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{claude.ScopeLocal: true}}, + }, } m.main.pendingOps = make(map[string]Operation) @@ -2064,8 +2076,10 @@ func TestApplyScopeDialogDeltaMixed(t *testing.T) { m.main.scopeDialog = scopeDialogState{ pluginID: "test@marketplace", // Originally installed at Project and Local, now checking User and unchecking Project - scopes: [3]bool{true, false, true}, - originalScopes: map[claude.Scope]bool{claude.ScopeProject: true, claude.ScopeLocal: true}, + scopes: [3]bool{true, false, true}, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{claude.ScopeProject: true, claude.ScopeLocal: true}}, + }, } m.main.pendingOps = make(map[string]Operation) @@ -2094,9 +2108,11 @@ func TestApplyScopeDialogDeltaNoChange(t *testing.T) { client := &mockClient{} m := NewModel(client, "/test/project") m.main.scopeDialog = scopeDialogState{ - pluginID: "test@marketplace", - scopes: [3]bool{true, false, false}, - originalScopes: map[claude.Scope]bool{claude.ScopeUser: true}, + pluginID: "test@marketplace", + scopes: [3]bool{true, false, false}, + targets: []scopeDialogTarget{ + {pluginID: "test@marketplace", originalScopes: map[claude.Scope]bool{claude.ScopeUser: true}}, + }, } m.main.pendingOps = make(map[string]Operation) m.main.pendingOps["test@marketplace"] = Operation{PluginID: "test@marketplace", Type: OpInstall} @@ -2109,40 +2125,6 @@ func TestApplyScopeDialogDeltaNoChange(t *testing.T) { } } -// TestApplyScopeDialogDeltaTargetlessFallback explicitly exercises the -// backward-compatible fallback in applyScopeDialogDelta: when the dialog has no -// targets slice, the delta is computed from the top-level pluginID/originalScopes -// fields instead. Production code always populates targets, so this guards the -// fallback that direct scopeDialogState assignments (e.g. older tests) rely on. -func TestApplyScopeDialogDeltaTargetlessFallback(t *testing.T) { - client := &mockClient{} - m := NewModel(client, "/test/project") - // No targets set — only the legacy top-level fields. - m.main.scopeDialog = scopeDialogState{ - pluginID: "legacy@marketplace", - scopes: [3]bool{true, false, false}, - originalScopes: map[claude.Scope]bool{}, - } - m.main.pendingOps = make(map[string]Operation) - - if len(m.main.scopeDialog.targets) != 0 { - t.Fatalf("precondition failed: targets should be empty, got %d", len(m.main.scopeDialog.targets)) - } - - m.applyScopeDialogDelta() - - op, ok := m.main.pendingOps["legacy@marketplace"] - if !ok { - t.Fatal("expected pending operation derived from the targetless fallback") - } - if op.Type != OpInstall { - t.Errorf("Type = %v, want OpInstall", op.Type) - } - if len(op.Scopes) != 1 || op.Scopes[0] != claude.ScopeUser { - t.Errorf("Scopes = %v, want [ScopeUser]", op.Scopes) - } -} - // TestRenderScopeDialog tests dialog rendering with checkbox display. // Verifies plugin-scope-mgmt.AC6.1 and AC6.4: Dialog renders with scope names and file paths. func TestRenderScopeDialog(t *testing.T) { @@ -2274,6 +2256,53 @@ func TestOpenScopeDialogForSelectedMultiSelectTargets(t *testing.T) { } } +// TestOpenScopeDialogForSelectedFocusedNotInBulk verifies that when the focused +// plugin is NOT part of the bulk selection, the dialog targets only the +// bulk-selected plugins and the checkboxes seed from the first target's installed +// scopes (alpha's) rather than the focused-but-unselected plugin's (gamma's). +func TestOpenScopeDialogForSelectedFocusedNotInBulk(t *testing.T) { + m, _ := testModelMultiPlugin(map[string][]claude.Scope{ + "alpha": {claude.ScopeUser}, // first target -> should seed checkboxes + "beta": {claude.ScopeProject}, + "gamma": {claude.ScopeLocal}, // focused but not bulk-selected + }) + // Bulk-select alpha and beta; gamma is intentionally excluded. + m.main.bulkSelected["alpha@marketplace"] = true + m.main.bulkSelected["beta@marketplace"] = true + // Focus gamma (index 3: header=0, alpha=1, beta=2, gamma=3). + m.selectedIdx = 3 + + m.openScopeDialogForSelected() + + if m.mode != ModeScopeDialog { + t.Fatalf("mode = %v, want ModeScopeDialog", m.mode) + } + + // Targets must be exactly the bulk selection, not the focused plugin. + if len(m.main.scopeDialog.targets) != 2 { + t.Fatalf("targets = %d, want 2 (only bulk-selected plugins)", len(m.main.scopeDialog.targets)) + } + gotIDs := map[string]bool{} + for _, target := range m.main.scopeDialog.targets { + gotIDs[target.pluginID] = true + } + if !gotIDs["alpha@marketplace"] || !gotIDs["beta@marketplace"] { + t.Errorf("targets = %v, want alpha and beta", gotIDs) + } + if gotIDs["gamma@marketplace"] { + t.Error("gamma should not be a target (focused but not bulk-selected)") + } + + // Checkboxes must reflect the first target (alpha: User) and NOT the focused + // gamma (Local). scopes indices: [0]=User, [1]=Project, [2]=Local. + if !m.main.scopeDialog.scopes[0] { + t.Error("User checkbox should be checked (alpha's installed scope seeds the dialog)") + } + if m.main.scopeDialog.scopes[2] { + t.Error("Local checkbox should be unchecked (gamma's scope must not seed the dialog)") + } +} + // TestOpenScopeDialogForSelectedFallsBackToFocused verifies the single-plugin UX: // with no bulk selection, S acts only on the focused plugin. func TestOpenScopeDialogForSelectedFallsBackToFocused(t *testing.T) { diff --git a/internal/tui/update.go b/internal/tui/update.go index 12d2efc..8126559 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -225,16 +225,7 @@ func (m *Model) updateScopeDialog(msg tea.Msg) (tea.Model, tea.Cmd) { func (m *Model) applyScopeDialogDelta() { dialog := &m.main.scopeDialog - targets := dialog.targets - if len(targets) == 0 { - // Backward-compatible fallback for callers that set pluginID directly - // without a targets slice. Production code always populates targets via - // openScopeDialogForTargets; this path is covered by - // TestApplyScopeDialogDeltaTargetlessFallback. - targets = []scopeDialogTarget{{pluginID: dialog.pluginID, originalScopes: dialog.originalScopes}} - } - - for _, target := range targets { + for _, target := range dialog.targets { m.applyScopeDeltaForTarget(target, dialog.scopes) } }