From 2ac088b388ae8d5fef973bfbb0f48dc2bea39234 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 23:14:57 +0800 Subject: [PATCH] feat: add contextual Target actions --- .../issues/05-contextual-target-actions.md | 30 ++- README.md | 2 +- internal/app/app.go | 225 ++++++++++++------ internal/app/contextual_actions_test.go | 150 +++++++++++- internal/app/coordination_test.go | 6 + internal/app/maintenance_test.go | 6 +- internal/app/operation_stream_test.go | 13 +- internal/app/progressive_detection_test.go | 6 +- internal/app/target_test.go | 8 + internal/tui/contextual_actions_test.go | 35 +++ internal/tui/dashboard_view.go | 20 +- internal/tui/foundation_test.go | 14 +- internal/tui/model.go | 27 +-- internal/tui/model_test.go | 38 +-- internal/tui/operation_coordination_test.go | 18 +- internal/tui/target_test.go | 24 +- 16 files changed, 428 insertions(+), 194 deletions(-) diff --git a/.scratch/tui-redesign/issues/05-contextual-target-actions.md b/.scratch/tui-redesign/issues/05-contextual-target-actions.md index 3036cdd..b5c3629 100644 --- a/.scratch/tui-redesign/issues/05-contextual-target-actions.md +++ b/.scratch/tui-redesign/issues/05-contextual-target-actions.md @@ -4,17 +4,21 @@ **Blocked by:** 04 — Unify operation coordination and status feedback. -**Status:** ready-for-agent +**Status:** done -- [ ] A Disabled Target offers Enable. -- [ ] An Enabled Target offers Disable. -- [ ] An External Target offers Apply Profile and explains that proxyctl will begin managing the configuration while preserving restorable state. -- [ ] A Drifted Target offers Resolve rather than silently choosing a direction. -- [ ] Drift Resolution offers Reapply Active Profile and Restore Original as separate intents. -- [ ] Reapply uses the Active Profile and returns truthful Enabled, Drifted, or failure state. -- [ ] Restore Original reuses existing ownership safeguards and refuses safely when restoration evidence is insufficient. -- [ ] No Adapter ownership, restoration, or cross-Target rollback semantics are weakened. -- [ ] Docker restart confirmation remains required for relevant actions and cancellation is the default focused choice. -- [ ] Routine reversible actions do not gain new confirmation dialogs. -- [ ] Action availability and disabled reasons are structured in the Application snapshot rather than inferred from rendered labels. -- [ ] Application and complete-Model tests cover all Target states, both Drift Resolution branches, missing Active Profile, ownership loss, confirmation, and cancellation without mutation. +- [x] A Disabled Target offers Enable. +- [x] An Enabled Target offers Disable. +- [x] An External Target offers Apply Profile and explains that proxyctl will begin managing the configuration while preserving restorable state. +- [x] A Drifted Target offers Resolve rather than silently choosing a direction. +- [x] Drift Resolution offers Reapply Active Profile and Restore Original as separate intents. +- [x] Reapply uses the Active Profile and returns truthful Enabled, Drifted, or failure state. +- [x] Restore Original reuses existing ownership safeguards and refuses safely when restoration evidence is insufficient. +- [x] No Adapter ownership, restoration, or cross-Target rollback semantics are weakened. +- [x] Docker restart confirmation remains required for relevant actions and cancellation is the default focused choice. +- [x] Routine reversible actions do not gain new confirmation dialogs. +- [x] Action availability and disabled reasons are structured in the Application snapshot rather than inferred from rendered labels. +- [x] Application and complete-Model tests cover all Target states, both Drift Resolution branches, missing Active Profile, ownership loss, confirmation, and cancellation without mutation. + +## Answer + +The Application snapshot now exposes contextual Target actions and Drift Resolution choices as structured availability plus disabled reasons. Dispatch revalidates every Target intent against that current structure immediately before confirmation or `SetDesired`, so stale, checking, failed, unavailable, profile-dependent, and ownership-sensitive intents fail without unsafe mutation. Reapply and Restore retain Adapter `SetDesired` semantics and existing restoration safeguards, while the complete Model preserves explicit resolution and safe-default confirmation/cancellation behavior. diff --git a/README.md b/README.md index a3096e5..73ad0a1 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ proxyctl Press `p` for Proxy Profile management, `n` to create a Profile, enter a name and proxy URL, and press Ctrl+S to save. Single mode applies one URL to HTTP, HTTPS, and ALL/SOCKS. Ctrl+T selects advanced mode for separate values. A bare `host:port` is accepted and normalized to `http://`. With the cursor in the Single proxy URL field, Ctrl+D runs the Protocol Probe, which detects whether the endpoint speaks HTTP or SOCKS5 and fills in the scheme. When one endpoint supports both protocols, the probe switches the form to advanced mode and fills HTTP/HTTPS with `http://` and ALL/SOCKS with `socks5://`. Bypass accepts hostnames, domain suffixes, IPv4, IPv6, and CIDR entries. Select the Profile and press `a` or Enter to make it the Active Profile. -The main panel shows the Active Profile. Use arrows or `j`/`k` to select an Available Target, Space to toggle it, `r` to refresh detection, `?` for details, and `q` or Escape to leave. A Target cannot be enabled without an Active Profile, but previously managed configuration can still be disabled. +The main panel shows the Active Profile. Use arrows or `j`/`k` to select an Available Target and Space to run its contextual action: Enable, Disable, Apply Profile, or Resolve. Drift Resolution explicitly offers Reapply Active Profile and Restore Original; restoration refuses safely if proxyctl no longer has sufficient ownership evidence. Press `r` to refresh detection, `u` to show unavailable Targets, `?` for Help, and `q` to leave. Applying or reapplying requires an Active Profile, while Restore Original remains available without one. Profiles are human-readable under the user's XDG configuration directory (normally `~/.config/proxyctl/profiles.json`) and are stored owner-only because proxy URLs may contain credentials. diff --git a/internal/app/app.go b/internal/app/app.go index 2cdfb1c..156534d 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -57,6 +57,44 @@ type TargetAction struct { DisabledReason string } +type targetActionPolicy struct { + kind TargetActionKind + intent IntentKind + state target.State + desired bool + primary bool + requiresProfile bool + operation string +} + +var targetActionPolicies = []targetActionPolicy{ + {kind: EnableTargetAction, intent: EnableTarget, state: target.Disabled, desired: true, primary: true, requiresProfile: true, operation: "enable"}, + {kind: DisableTargetAction, intent: DisableTarget, state: target.Enabled, primary: true, operation: "disable"}, + {kind: ApplyProfileTargetAction, intent: ApplyProfileToTarget, state: target.External, desired: true, primary: true, requiresProfile: true, operation: "apply-Profile"}, + {kind: ResolveDriftTargetAction, state: target.Drifted, primary: true}, + {kind: ReapplyActiveProfileAction, intent: ReapplyActiveProfile, state: target.Drifted, desired: true, requiresProfile: true, operation: "reapply-Active-Profile"}, + {kind: RestoreOriginalAction, intent: RestoreOriginal, state: target.Drifted, operation: "restore-original"}, +} + +func targetActionPolicyForKind(kind TargetActionKind) (targetActionPolicy, bool) { + for _, policy := range targetActionPolicies { + if policy.kind == kind { + return policy, true + } + } + return targetActionPolicy{}, false +} + +// Intent creates the Application intent represented by a structured Target +// action. Presentation layers use this as the sole action-to-intent boundary. +func (kind TargetActionKind) Intent(targetID string) (Intent, bool) { + policy, ok := targetActionPolicyForKind(kind) + if !ok || policy.intent == 0 { + return Intent{}, false + } + return Intent{Kind: policy.intent, TargetID: targetID}, true +} + type TargetDetectionPhase uint8 const ( @@ -193,13 +231,16 @@ func (a *Application) snapshotLocked() Snapshot { } func contextualTargetActions(phase TargetDetectionPhase, available bool, state target.State, hasActiveProfile bool) (TargetAction, []TargetAction) { - primary := TargetAction{Kind: primaryTargetActionKind(state), Available: true} + primaryKind := primaryTargetActionKind(state) + primary := TargetAction{Kind: primaryKind, Available: primaryKind != 0} + if primaryKind == 0 { + primary.DisabledReason = "Target actions are unavailable because this Target state is unknown" + } if reason := targetActionDisabledReason(phase, available); reason != "" { primary.Available = false primary.DisabledReason = reason } - needsProfile := state == target.Disabled || state == target.External - if primary.Available && needsProfile && !hasActiveProfile { + if policy, ok := targetActionPolicyForKind(primaryKind); ok && primary.Available && policy.requiresProfile && !hasActiveProfile { primary.Available = false primary.DisabledReason = "This action is unavailable because there is no Active Profile; open Profile management first" } @@ -207,36 +248,32 @@ func contextualTargetActions(phase TargetDetectionPhase, available bool, state t return primary, nil } - resolution := []TargetAction{ - {Kind: ReapplyActiveProfileAction, Available: true}, - {Kind: RestoreOriginalAction, Available: true}, - } - if !hasActiveProfile { - resolution[0].Available = false - resolution[0].DisabledReason = "Reapply Active Profile is unavailable because there is no Active Profile; open Profile management first" - } - if reason := targetActionDisabledReason(phase, available); reason != "" { - for i := range resolution { - resolution[i].Available = false - resolution[i].DisabledReason = reason + var resolution []TargetAction + for _, policy := range targetActionPolicies { + if policy.state != target.Drifted || policy.primary || policy.intent == 0 { + continue } + action := TargetAction{Kind: policy.kind, Available: true} + if policy.requiresProfile && !hasActiveProfile { + action.Available = false + action.DisabledReason = "Reapply Active Profile is unavailable because there is no Active Profile; open Profile management first" + } + if reason := targetActionDisabledReason(phase, available); reason != "" { + action.Available = false + action.DisabledReason = reason + } + resolution = append(resolution, action) } return primary, resolution } func primaryTargetActionKind(state target.State) TargetActionKind { - switch state { - case target.Disabled: - return EnableTargetAction - case target.Enabled: - return DisableTargetAction - case target.External: - return ApplyProfileTargetAction - case target.Drifted: - return ResolveDriftTargetAction - default: - return 0 + for _, policy := range targetActionPolicies { + if policy.primary && policy.state == state { + return policy.kind + } } + return 0 } func targetActionDisabledReason(phase TargetDetectionPhase, available bool) string { @@ -352,7 +389,7 @@ func (a *Application) dispatch(intent Intent, publish func(Result)) Result { return Result{Snapshot: a.Snapshot(), Err: fmt.Errorf("reload Profile state for mutation: %w", err)} } a.replaceState(fresh) - if isTargetMutationIntent(intent.Kind) || intent.Kind == ActivateProfile { + if intent.Kind == ActivateProfile { a.refreshInspections() } if isTargetMutationIntent(intent.Kind) { @@ -584,6 +621,23 @@ func (a *Application) replaceInspection(id string, inspection target.Inspection) a.inspections[id] = inspection } +// publishTargetDetection atomically replaces all availability, inspection, and +// failure fields from one targeted check. Detect runs before this method, so no +// Application state mutex is held while adapter code executes. +func (a *Application) publishTargetDetection(id string, detection target.Detection) Snapshot { + a.stateMu.Lock() + defer a.stateMu.Unlock() + a.detectionGeneration++ + a.availabilities[id] = detection.Availability + a.inspections[id] = detection.Inspection + a.detectionErr[id] = detection.Err + a.detectionPhase[id] = TargetReady + if detection.Err != nil { + a.detectionPhase[id] = TargetDetectionFailed + } + return a.snapshotLocked() +} + func (a *Application) replaceInspections(inspections map[string]target.Inspection) { a.stateMu.Lock() defer a.stateMu.Unlock() @@ -672,29 +726,66 @@ func (a *Application) refreshInspections() { } func isTargetMutationIntent(kind IntentKind) bool { - switch kind { - case SetTargetDesired, EnableTarget, DisableTarget, ApplyProfileToTarget, ReapplyActiveProfile, RestoreOriginal: + if kind == SetTargetDesired { return true - default: - return false } + for _, policy := range targetActionPolicies { + if policy.intent == kind { + return true + } + } + return false } -func contextualIntentSemantics(kind IntentKind) (expected target.State, enabled bool, ok bool) { - switch kind { - case EnableTarget: - return target.Disabled, true, true - case DisableTarget: - return target.Enabled, false, true - case ApplyProfileToTarget: - return target.External, true, true - case ReapplyActiveProfile: - return target.Drifted, true, true - case RestoreOriginal: - return target.Drifted, false, true - default: - return "", false, false +func requestedTargetAction(intent Intent, selected TargetSnapshot) (TargetActionKind, bool) { + if intent.Kind == SetTargetDesired { + return legacyTargetAction(intent.Enabled, selected.State) } + for _, policy := range targetActionPolicies { + if policy.intent == intent.Kind { + return policy.kind, true + } + } + return 0, false +} + +// legacyTargetAction is the only compatibility boundary for the former +// SetTargetDesired boolean contract. All current callers use TargetActionKind. +func legacyTargetAction(enabled bool, state target.State) (TargetActionKind, bool) { + for _, policy := range targetActionPolicies { + if policy.intent != 0 && policy.state == state && policy.desired == enabled { + return policy.kind, true + } + } + return 0, false +} + +func structuredTargetAction(selected TargetSnapshot, kind TargetActionKind) (TargetAction, bool) { + if selected.PrimaryAction.Kind == kind { + return selected.PrimaryAction, true + } + for _, action := range selected.DriftResolutionActions { + if action.Kind == kind { + return action, true + } + } + return TargetAction{}, false +} + +func validateTargetAction(intent Intent, selected TargetSnapshot) (targetActionPolicy, error) { + kind, ok := requestedTargetAction(intent, selected) + if !ok { + return targetActionPolicy{}, fmt.Errorf("Target %q action is no longer available because its state is %s", selected.ID, selected.State) + } + action, ok := structuredTargetAction(selected, kind) + if !ok { + return targetActionPolicy{}, fmt.Errorf("Target %q action is no longer available because its state is %s", selected.ID, selected.State) + } + if !action.Available { + return targetActionPolicy{}, errors.New(action.DisabledReason) + } + policy, _ := targetActionPolicyForKind(kind) + return policy, nil } func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Result { @@ -709,29 +800,28 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu return Result{Snapshot: a.Snapshot(), Err: fmt.Errorf("Target %q does not exist", intent.TargetID)} } selected := a.targets[selectedIndex] - enabled := intent.Enabled - if expected, contextualEnabled, contextual := contextualIntentSemantics(intent.Kind); contextual { - actual := a.inspection(intent.TargetID).State - if actual != expected { - return Result{Snapshot: a.Snapshot(), Err: fmt.Errorf("Target %q action is no longer available because its state is %s", intent.TargetID, actual)} - } - enabled = contextualEnabled - } active := a.activeProfile() - if enabled && active == nil { - return Result{Snapshot: a.Snapshot(), Err: errors.New("no Active Profile; open Profile management before enabling a Target")} + // This targeted check runs inside dispatch serialization and immediately + // before action validation. Adapter calls happen without stateMu; the full + // outcome is then published under one lock before policy is derived. + detection := detectTarget(selected, active) + snapshot := a.publishTargetDetection(intent.TargetID, detection) + selectedSnapshot := snapshot.Targets[selectedIndex] + policy, err := validateTargetAction(intent, selectedSnapshot) + if err != nil { + return Result{Snapshot: snapshot, Err: err} } - if warning := selected.Confirmation(enabled); warning != "" && !intent.Confirm { + if warning := selected.Confirmation(policy.desired); warning != "" && !intent.Confirm { confirmation := &Confirmation{Title: "Confirm " + a.targetDescriptions[selectedIndex].Name + " Target change", Summary: warning, ConfirmLabel: "Continue", CancelLabel: "Cancel", HighImpact: true} return Result{Snapshot: a.Snapshot(), ConfirmationRequest: confirmation, Status: OperationStatus{Summary: confirmation.Title, TargetID: intent.TargetID}} } if publish != nil { - publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{{TargetID: intent.TargetID, Phase: TargetProgressRunning, State: a.inspection(intent.TargetID).State}}, + publish(Result{Snapshot: snapshot, Progress: []TargetProgress{{TargetID: intent.TargetID, Phase: TargetProgressRunning, State: selectedSnapshot.State}}, Status: OperationStatus{Summary: "Updating " + a.targetDescriptions[selectedIndex].Name + " Target", TargetID: intent.TargetID}}) } - result := selected.SetDesired(context.Background(), enabled, active) + result := selected.SetDesired(context.Background(), policy.desired, active) inspection := result.Inspection if result.Err != nil { inspection = selected.Inspect(context.Background(), active) @@ -742,8 +832,7 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu outcome, detail = "failure", result.Err.Error() } if a.logger != nil { - operation := targetOperationName(intent.Kind, enabled) - _ = a.logger.Write(LogEntry{Time: time.Now(), Operation: operation, Target: intent.TargetID, Result: outcome, Detail: RedactLogDetail(detail)}) + _ = a.logger.Write(LogEntry{Time: time.Now(), Operation: policy.operation, Target: intent.TargetID, Result: outcome, Detail: RedactLogDetail(detail)}) } notice := "" progressPhase := TargetProgressSucceeded @@ -759,22 +848,6 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu return Result{Snapshot: a.Snapshot(), Notice: notice, Status: status, Progress: []TargetProgress{progress}, FirstFailedTargetID: map[bool]string{true: intent.TargetID}[result.Err != nil], Err: result.Err} } -func targetOperationName(kind IntentKind, enabled bool) string { - switch kind { - case ApplyProfileToTarget: - return "apply-Profile" - case ReapplyActiveProfile: - return "reapply-Active-Profile" - case RestoreOriginal: - return "restore-original" - default: - if enabled { - return "enable" - } - return "disable" - } -} - func create(state profile.State, input profile.Profile) (profile.State, string, error) { p, err := profile.ValidateAndNormalize(input) if err != nil { diff --git a/internal/app/contextual_actions_test.go b/internal/app/contextual_actions_test.go index b6d61a2..a21f08b 100644 --- a/internal/app/contextual_actions_test.go +++ b/internal/app/contextual_actions_test.go @@ -1,6 +1,7 @@ package app_test import ( + "context" "errors" "path/filepath" "strings" @@ -53,6 +54,24 @@ func TestApplicationSnapshotStructuresContextualTargetActions(t *testing.T) { } } +func TestApplicationSnapshotMakesUnknownTargetStateStructurallyUnavailable(t *testing.T) { + adapter := &fakeTarget{available: true} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) + if err != nil { + t.Fatal(err) + } + createAndActivate(t, application) + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } + + got := application.Snapshot().Targets[0].PrimaryAction + const reason = "Target actions are unavailable because this Target state is unknown" + if got.Kind != 0 || got.Available || got.DisabledReason != reason { + t.Fatalf("unknown-state action = %#v, want structurally unavailable with reason %q", got, reason) + } +} + func TestApplicationDispatchesExplicitContextualTargetIntents(t *testing.T) { tests := []struct { name string @@ -132,6 +151,57 @@ func TestApplicationReapplyPublishesTruthfulDriftWhenAdapterCannotConverge(t *te } } +type changingAvailabilityTarget struct { + *fakeTarget + detections int +} + +func (f *changingAvailabilityTarget) Detect(context.Context, *profile.Profile) target.Detection { + f.detections++ + available := f.detections == 1 + return target.Detection{ + Availability: target.Availability{Available: available, Evidence: map[bool]string{true: "available", false: "no longer available"}[available]}, + Inspection: target.Inspection{State: target.External, Evidence: "actual state"}, + } +} + +func TestApplicationRechecksAvailabilityImmediatelyBeforeTargetMutation(t *testing.T) { + base := &fakeTarget{available: true, state: target.External} + adapter := &changingAvailabilityTarget{fakeTarget: base} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) + if err != nil { + t.Fatal(err) + } + createAndActivate(t, application) + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } + if before := application.Snapshot().Targets[0]; !before.Available || !before.PrimaryAction.Available { + t.Fatalf("initial detection = %#v, want available action", before) + } + + result := application.Dispatch(app.Intent{Kind: app.ApplyProfileToTarget, TargetID: adapter.ID()}) + const reason = "Target actions are unavailable because this Target is unavailable" + if result.Err == nil || result.Err.Error() != reason || len(base.calls) != 0 { + t.Fatalf("available-to-unavailable result = %#v, SetDesired calls = %#v", result, base.calls) + } + if got := result.Snapshot.Targets[0]; got.Available || got.AvailabilityEvidence != "no longer available" || got.PrimaryAction.Available || got.PrimaryAction.DisabledReason != reason { + t.Fatalf("published targeted detection = %#v", got) + } +} + +type failedDetectionTarget struct { + *fakeTarget +} + +func (f failedDetectionTarget) Detect(context.Context, *profile.Profile) target.Detection { + return target.Detection{ + Availability: target.Availability{Available: true, Evidence: "detected"}, + Inspection: target.Inspection{State: f.state, Evidence: "actual state"}, + Err: errors.New("detection failed"), + } +} + func TestApplicationRejectsStructuredUnavailableActionWithoutAdapterMutation(t *testing.T) { adapter := &fakeTarget{available: false, state: target.External} application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) @@ -153,18 +223,88 @@ func TestApplicationRejectsStructuredUnavailableActionWithoutAdapterMutation(t * } } -func TestApplicationRejectsProfileDependentIntentWithoutActiveProfile(t *testing.T) { - adapter := &fakeTarget{available: true, state: target.External} +func TestApplicationValidatesEveryStructuredDisabledReasonBeforeAdapterMutation(t *testing.T) { + assertRejected := func(t *testing.T, application *app.Application, adapter *fakeTarget, intent app.Intent, reason string) { + t.Helper() + adapter.calls = nil + result := application.Dispatch(intent) + if result.Err == nil || !strings.Contains(result.Err.Error(), reason) || len(adapter.calls) != 0 { + t.Fatalf("disabled action result = %#v, calls = %#v, want reason containing %q", result, adapter.calls, reason) + } + } + + t.Run("failed check", func(t *testing.T) { + adapter := &fakeTarget{available: true, state: target.Disabled} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{failedDetectionTarget{fakeTarget: adapter}}) + if err != nil { + t.Fatal(err) + } + createAndActivate(t, application) + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err == nil { + t.Fatal("Target check unexpectedly succeeded") + } + assertRejected(t, application, adapter, app.Intent{Kind: app.EnableTarget, TargetID: adapter.ID()}, "check failed") + }) + + t.Run("unavailable Drift Resolution", func(t *testing.T) { + adapter := &fakeTarget{available: false, state: target.Drifted} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) + if err != nil { + t.Fatal(err) + } + createAndActivate(t, application) + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } + assertRejected(t, application, adapter, app.Intent{Kind: app.RestoreOriginal, TargetID: adapter.ID()}, "unavailable") + }) +} + +func TestApplicationRejectsProfileDependentIntentsWithoutActiveProfile(t *testing.T) { + for _, test := range []struct { + name string + state target.State + intent app.IntentKind + }{ + {name: "Apply Profile", state: target.External, intent: app.ApplyProfileToTarget}, + {name: "Reapply Active Profile", state: target.Drifted, intent: app.ReapplyActiveProfile}, + } { + t.Run(test.name, func(t *testing.T) { + adapter := &fakeTarget{available: true, state: test.state} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) + if err != nil { + t.Fatal(err) + } + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } + result := application.Dispatch(app.Intent{Kind: test.intent, TargetID: adapter.ID()}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "no Active Profile") || len(adapter.calls) != 0 { + t.Fatalf("missing Active Profile result = %#v, calls=%#v", result, adapter.calls) + } + }) + } +} + +func TestApplicationRevalidatesRestoreAfterOwnershipSensitiveStateChange(t *testing.T) { + adapter := &fakeTarget{available: true, state: target.Drifted} application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) if err != nil { t.Fatal(err) } + createAndActivate(t, application) if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { t.Fatal(result.Err) } - result := application.Dispatch(app.Intent{Kind: app.ApplyProfileToTarget, TargetID: adapter.ID()}) - if result.Err == nil || !strings.Contains(result.Err.Error(), "no Active Profile") || len(adapter.calls) != 0 { - t.Fatalf("missing Active Profile result = %#v, calls=%#v", result, adapter.calls) + + // Model ownership loss as a fresh inspection becoming External between the + // displayed Drift Resolution and dispatch. The stale Restore intent must not + // reach SetDesired, where it could otherwise overwrite external state. + adapter.state = target.External + adapter.calls = nil + result := application.Dispatch(app.Intent{Kind: app.RestoreOriginal, TargetID: adapter.ID()}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "External") || len(adapter.calls) != 0 || result.Snapshot.Targets[0].State != target.External { + t.Fatalf("stale Restore result = %#v, calls = %#v", result, adapter.calls) } } diff --git a/internal/app/coordination_test.go b/internal/app/coordination_test.go index 358929e..e8deed0 100644 --- a/internal/app/coordination_test.go +++ b/internal/app/coordination_test.go @@ -87,6 +87,7 @@ func TestSecondApplicationUsesPersistedActiveProfileChangedByFirstApplication(t if got := second.Snapshot().ActiveProfile; got != "old" { t.Fatalf("second Application starting Active Profile = %q, want old", got) } + refreshApplicationTargets(t, second) if result := first.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "new"}); result.Err != nil { t.Fatalf("first Application activates new Profile: %v", result.Err) @@ -119,6 +120,7 @@ func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing. if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) for _, id := range []string{"first", "second"} { if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: id, Enabled: true}); result.Err != nil { t.Fatal(result.Err) @@ -188,6 +190,7 @@ func TestApplicationCoordinatesMultipleRealAndFakeHermeticAdaptersThroughPartial if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) for _, id := range []string{shellAdapter.ID(), dockerAdapter.ID(), failure.ID()} { if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: id, Enabled: true}); result.Err != nil { t.Fatalf("enable %s: %v", id, result.Err) @@ -229,6 +232,7 @@ func TestActiveProfileProgressReusesAvailabilityCache(t *testing.T) { if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}); result.Err != nil { t.Fatal(result.Err) } @@ -264,6 +268,7 @@ func TestActiveProfileSwitchStreamsApplyingAndCompletionForEachTarget(t *testing if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}); result.Err != nil { t.Fatal(result.Err) } @@ -309,6 +314,7 @@ func TestDeletingFinalProfileLeavesManagedTargetsDriftedAndDisableStillRestoresE if activated := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "only"}); activated.Err != nil { t.Fatal(activated.Err) } + refreshApplicationTargets(t, application) if enabled := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: adapter.ID(), Enabled: true}); enabled.Err != nil { t.Fatal(enabled.Err) } diff --git a/internal/app/maintenance_test.go b/internal/app/maintenance_test.go index d4b52ca..9827154 100644 --- a/internal/app/maintenance_test.go +++ b/internal/app/maintenance_test.go @@ -111,7 +111,7 @@ func TestDoctorReportsReadOnlyChecksEvidenceAndRedactsCredentials(t *testing.T) } func TestApplicationSerializesAndLogsTargetMutations(t *testing.T) { - adapter := &maintenanceTarget{id: "fake", available: target.Availability{Available: true}, inspection: target.Inspection{State: target.External}} + adapter := &maintenanceTarget{id: "fake", available: target.Availability{Available: true}, inspection: target.Inspection{State: target.Disabled}} lock := &recordingLock{} log := &recordingLog{} application, err := app.NewWithTargetsAndRuntime(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}, lock, log) @@ -124,12 +124,14 @@ func TestApplicationSerializesAndLogsTargetMutations(t *testing.T) { if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "office"}); result.Err != nil { t.Fatal(result.Err) } + adapter.inspection = target.Inspection{State: target.Enabled} + refreshApplicationTargets(t, application) adapter.disableErr = errors.New("failed http://alice:secret@proxy.test/private") result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "fake", Enabled: false}) if result.Err == nil { t.Fatal("Target failure reported success") } - if lock.acquired != 3 || lock.held != 0 { + if lock.acquired != 4 || lock.held != 0 { t.Fatalf("all mutation lock acquired=%d held=%d", lock.acquired, lock.held) } if len(log.entries) != 1 || log.entries[0].Operation != "disable" || strings.Contains(log.entries[0].Detail, "secret") || strings.Contains(log.entries[0].Detail, "proxy.test/private") { diff --git a/internal/app/operation_stream_test.go b/internal/app/operation_stream_test.go index d311c48..f4fb329 100644 --- a/internal/app/operation_stream_test.go +++ b/internal/app/operation_stream_test.go @@ -44,7 +44,11 @@ func (t *coordinatedOperationTarget) Detect(ctx context.Context, active *profile t.inDetection = true t.mu.Unlock() if t.detectionStarted != nil { - close(t.detectionStarted) + select { + case <-t.detectionStarted: + default: + close(t.detectionStarted) + } } if t.detectionRelease != nil { <-t.detectionRelease @@ -91,6 +95,7 @@ func TestInteractiveOperationStreamSerializesMutationAndPublishesStructuredProgr if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "office"}); result.Err != nil { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) operation := application.StartOperation(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}) running := <-operation.Updates @@ -162,7 +167,6 @@ func TestStartOperationReturnsBeforeDispatchSerializationIsAvailable(t *testing. adapter := &coordinatedOperationTarget{ id: "shell", state: target.Disabled, started: make(chan struct{}), release: make(chan struct{}), - detectionStarted: make(chan struct{}), detectionRelease: make(chan struct{}), } application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) if err != nil { @@ -174,12 +178,15 @@ func TestStartOperationReturnsBeforeDispatchSerializationIsAvailable(t *testing. if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "office"}); result.Err != nil { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) mutationDone := make(chan app.Result, 1) go func() { mutationDone <- application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}) }() <-adapter.started // barrier: synchronous Dispatch owns dispatch serialization + adapter.detectionStarted = make(chan struct{}) + adapter.detectionRelease = make(chan struct{}) returned := make(chan app.OperationStream, 1) go func() { returned <- application.StartOperation(app.Intent{Kind: app.RefreshTargets}) }() @@ -213,6 +220,7 @@ func TestInteractiveOperationStreamUsesStructuredConfirmation(t *testing.T) { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) operation := application.StartOperation(app.Intent{Kind: app.SetTargetDesired, TargetID: adapter.ID(), Enabled: true}) update := <-operation.Updates confirmation := update.Confirmation() @@ -239,6 +247,7 @@ func TestInteractiveProfileTransitionReportsFirstFailureWithoutRollback(t *testi if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil { t.Fatal(result.Err) } + refreshApplicationTargets(t, application) for _, id := range []string{"first", "second"} { if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: id, Enabled: true}); result.Err != nil { t.Fatal(result.Err) diff --git a/internal/app/progressive_detection_test.go b/internal/app/progressive_detection_test.go index b4371b6..9b4b8c9 100644 --- a/internal/app/progressive_detection_test.go +++ b/internal/app/progressive_detection_test.go @@ -190,7 +190,11 @@ func (t *mutationInteractionTarget) SetDesired(context.Context, bool, *profile.P return target.OperationResult{Inspection: target.Inspection{State: target.Enabled, Evidence: "mutation completed"}} } func (t *mutationInteractionTarget) Detect(context.Context, *profile.Profile) target.Detection { - close(t.started) + select { + case <-t.started: + default: + close(t.started) + } <-t.release return target.Detection{ Availability: target.Availability{Available: true, Evidence: "stale detection"}, diff --git a/internal/app/target_test.go b/internal/app/target_test.go index 25c4ebc..84185d1 100644 --- a/internal/app/target_test.go +++ b/internal/app/target_test.go @@ -86,6 +86,7 @@ func TestApplicationRequiresTargetConfirmationBeforeMutation(t *testing.T) { t.Fatal(err) } createAndActivate(t, application) + refreshApplicationTargets(t, application) warning := application.Dispatch(app.Intent{Kind: app.EnableTarget, TargetID: "docker-daemon"}) if warning.ConfirmationRequest == nil || warning.Err != nil || len(adapter.calls) != 0 || warning.Snapshot.Targets[0].State != target.Disabled { @@ -119,6 +120,13 @@ func TestApplicationHidesUnavailableTargetButKeepsExplanation(t *testing.T) { } } +func refreshApplicationTargets(t *testing.T, application *app.Application) { + t.Helper() + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } +} + func createAndActivate(t *testing.T, application *app.Application) { t.Helper() if result := application.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{Name: "office", SingleURL: "http://proxy.example:8080"}}); result.Err != nil { diff --git a/internal/tui/contextual_actions_test.go b/internal/tui/contextual_actions_test.go index 9310e65..7d74fe8 100644 --- a/internal/tui/contextual_actions_test.go +++ b/internal/tui/contextual_actions_test.go @@ -83,6 +83,24 @@ func TestCompleteModelOffersBothExplicitDriftResolutionIntents(t *testing.T) { } } +func TestCompleteModelRendersUnknownTargetStateAsStructurallyUnavailable(t *testing.T) { + const reason = "Target actions are unavailable because this Target state is unknown" + initial := completeTargetSnapshot("", app.TargetAction{DisabledReason: reason}) + initial.Targets[0].PrimaryAction.Available = false + module := &targetModule{snapshot: initial} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + + view := model.View() + if !strings.Contains(view, "Action: Unavailable") || !strings.Contains(view, "Action unavailable:") || !strings.Contains(view, "state is unknown") || !strings.Contains(view, "Space: Unavailable disabled") { + t.Fatalf("unknown Target action was not structurally unavailable:\n%s", view) + } + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if command != nil || len(module.intents) != 0 || model.operation.status.Summary != reason { + t.Fatalf("unknown Target action dispatched: intents=%#v status=%q\n%s", module.intents, model.operation.status.Summary, model.View()) + } +} + func TestCompleteModelUsesStructuredDisabledReasonWithoutInferringFromState(t *testing.T) { initial := completeTargetSnapshot(target.Disabled, app.TargetAction{ Kind: app.EnableTargetAction, DisabledReason: "No Active Profile is selected; create or select one before enabling", @@ -100,6 +118,23 @@ func TestCompleteModelUsesStructuredDisabledReasonWithoutInferringFromState(t *t } } +func TestCompleteModelPressingUnavailableTargetActionShowsExactReasonAndDispatchesNoIntent(t *testing.T) { + const reason = "GNOME settings schema is unavailable for the owning user" + initial := completeTargetSnapshot(target.External, app.TargetAction{ + Kind: app.ApplyProfileTargetAction, Available: false, DisabledReason: reason, + }) + initial.Targets[0].Available = false + module := &targetModule{snapshot: initial} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + model = updateKey(model, "u") + + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if command != nil || len(module.intents) != 0 || model.operation.status.Summary != reason { + t.Fatalf("unavailable action: command=%v intents=%#v status=%q", command != nil, module.intents, model.operation.status.Summary) + } +} + func TestCompleteModelRestoreOwnershipLossFailsTruthfully(t *testing.T) { initial := completeTargetSnapshot(target.Drifted, app.TargetAction{Kind: app.ResolveDriftTargetAction, Available: true}) initial.Targets[0].DriftResolutionActions = []app.TargetAction{ diff --git a/internal/tui/dashboard_view.go b/internal/tui/dashboard_view.go index ac2a8fb..9cd89b0 100644 --- a/internal/tui/dashboard_view.go +++ b/internal/tui/dashboard_view.go @@ -142,17 +142,15 @@ func (p terminalPresentation) targetDetails(item app.TargetSnapshot) string { var details strings.Builder details.WriteString(p.theme.section.Render(item.Description.Name)) fmt.Fprintf(&details, "\nState: %s\nEvidence: %s", state, evidence) - if item.PrimaryAction.Kind != 0 { - fmt.Fprintf(&details, "\nAction: %s", targetActionLabel(item.PrimaryAction.Kind)) - if !item.PrimaryAction.Available && item.PrimaryAction.DisabledReason != "" { - fmt.Fprintf(&details, "\nAction unavailable: %s", item.PrimaryAction.DisabledReason) - } - switch item.PrimaryAction.Kind { - case app.ApplyProfileTargetAction: - details.WriteString("\nApply Profile means proxyctl will begin managing this configuration while preserving restorable state.") - case app.ResolveDriftTargetAction: - details.WriteString("\nResolve opens Drift Resolution without choosing a recovery direction for you.") - } + fmt.Fprintf(&details, "\nAction: %s", targetActionLabel(item.PrimaryAction.Kind)) + if !item.PrimaryAction.Available && item.PrimaryAction.DisabledReason != "" { + fmt.Fprintf(&details, "\nAction unavailable: %s", item.PrimaryAction.DisabledReason) + } + switch item.PrimaryAction.Kind { + case app.ApplyProfileTargetAction: + details.WriteString("\nApply Profile means proxyctl will begin managing this configuration while preserving restorable state.") + case app.ResolveDriftTargetAction: + details.WriteString("\nResolve opens Drift Resolution without choosing a recovery direction for you.") } if item.DetectionPhase == app.TargetDetectionFailed && item.DetectionErr != nil { fmt.Fprintf(&details, "\nCheck error: %s", item.DetectionErr) diff --git a/internal/tui/foundation_test.go b/internal/tui/foundation_test.go index 49113e9..deed102 100644 --- a/internal/tui/foundation_test.go +++ b/internal/tui/foundation_test.go @@ -245,11 +245,11 @@ func productionScaleSnapshot() app.Snapshot { ActiveProfile: "office", Profiles: []profile.Profile{{Name: "office", Mode: profile.SingleMode, SingleURL: "http://proxy.example:8080"}}, Targets: []app.TargetSnapshot{ - {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell summary", Details: strings.Repeat("Shell detail. ", 30)}, Available: true, State: target.Enabled, Evidence: strings.Repeat("Shell evidence. ", 20)}, - {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT summary", Details: strings.Repeat("APT detail. ", 30)}, Available: true, State: target.Disabled, Evidence: strings.Repeat("APT evidence. ", 20)}, - {ID: "docker-containers", Description: target.Description{Name: "Docker containers/builds", Summary: "Docker client summary", Details: strings.Repeat("Docker client detail. ", 30)}, Available: true, State: target.External, Evidence: strings.Repeat("Docker client evidence. ", 20)}, - {ID: "gnome", Description: target.Description{Name: "GNOME", Summary: "GNOME summary", Details: strings.Repeat("GNOME detail. ", 30)}, Available: true, State: target.Enabled, Evidence: strings.Repeat("GNOME evidence. ", 20)}, - {ID: "docker-daemon", Description: target.Description{Name: "Docker daemon", Summary: "Docker daemon selected summary", Details: strings.Repeat("Docker daemon selected detail. ", 30)}, Available: true, State: target.Drifted, Evidence: strings.Repeat("Docker daemon selected evidence. ", 20)}, + {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell summary", Details: strings.Repeat("Shell detail. ", 30)}, Available: true, State: target.Enabled, Evidence: strings.Repeat("Shell evidence. ", 20), PrimaryAction: app.TargetAction{Kind: app.DisableTargetAction, Available: true}}, + {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT summary", Details: strings.Repeat("APT detail. ", 30)}, Available: true, State: target.Disabled, Evidence: strings.Repeat("APT evidence. ", 20), PrimaryAction: app.TargetAction{Kind: app.EnableTargetAction, Available: true}}, + {ID: "docker-containers", Description: target.Description{Name: "Docker containers/builds", Summary: "Docker client summary", Details: strings.Repeat("Docker client detail. ", 30)}, Available: true, State: target.External, Evidence: strings.Repeat("Docker client evidence. ", 20), PrimaryAction: app.TargetAction{Kind: app.ApplyProfileTargetAction, Available: true}}, + {ID: "gnome", Description: target.Description{Name: "GNOME", Summary: "GNOME summary", Details: strings.Repeat("GNOME detail. ", 30)}, Available: true, State: target.Enabled, Evidence: strings.Repeat("GNOME evidence. ", 20), PrimaryAction: app.TargetAction{Kind: app.DisableTargetAction, Available: true}}, + {ID: "docker-daemon", Description: target.Description{Name: "Docker daemon", Summary: "Docker daemon selected summary", Details: strings.Repeat("Docker daemon selected detail. ", 30)}, Available: true, State: target.Drifted, Evidence: strings.Repeat("Docker daemon selected evidence. ", 20), PrimaryAction: app.TargetAction{Kind: app.ResolveDriftTargetAction, Available: true}, DriftResolutionActions: []app.TargetAction{{Kind: app.ReapplyActiveProfileAction, Available: true}, {Kind: app.RestoreOriginalAction, Available: true}}}, }, } } @@ -261,8 +261,8 @@ func responsiveLayoutModule() *recordingModule { ActiveProfile: "office", Profiles: []profile.Profile{{Name: "office", Mode: profile.SingleMode, SingleURL: "http://proxy.example:8080"}}, Targets: []app.TargetSnapshot{ - {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell summary", Details: "Shell details"}, Available: true, State: target.Disabled, Evidence: "Shell evidence"}, - {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT summary", Details: "APT details"}, Available: true, State: target.External, Evidence: "APT evidence"}, + {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell summary", Details: "Shell details"}, Available: true, State: target.Disabled, Evidence: "Shell evidence", PrimaryAction: app.TargetAction{Kind: app.EnableTargetAction, Available: true}}, + {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT summary", Details: "APT details"}, Available: true, State: target.External, Evidence: "APT evidence", PrimaryAction: app.TargetAction{Kind: app.ApplyProfileTargetAction, Available: true}}, }, }} } diff --git a/internal/tui/model.go b/internal/tui/model.go index 9fdaca3..1f25710 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -242,8 +242,6 @@ type mutationAvailability struct { // action from Target state or display text. func targetMutationAvailability(selected app.TargetSnapshot, selectedOK bool, operationBlocked bool) mutationAvailability { switch { - case operationBlocked: - return mutationAvailability{reason: "Wait for the current mutation to finish before starting another modification", compactReason: "mutation in progress"} case !selectedOK: return mutationAvailability{reason: "Target mutation is disabled because no Target is selected", compactReason: "no Target selected"} case !selected.PrimaryAction.Available: @@ -252,6 +250,8 @@ func targetMutationAvailability(selected app.TargetSnapshot, selectedOK bool, op reason = "This Target action is unavailable" } return mutationAvailability{reason: reason, compactReason: "unavailable"} + case operationBlocked: + return mutationAvailability{reason: "Wait for the current mutation to finish before starting another modification", compactReason: "mutation in progress"} default: return mutationAvailability{available: true} } @@ -266,25 +266,6 @@ func driftResolutionAction(selected app.TargetSnapshot, kind app.TargetActionKin return app.TargetAction{Kind: kind}, false } -func targetIntent(kind app.TargetActionKind, targetID string) (app.Intent, bool) { - intent := app.Intent{TargetID: targetID} - switch kind { - case app.EnableTargetAction: - intent.Kind = app.EnableTarget - case app.DisableTargetAction: - intent.Kind = app.DisableTarget - case app.ApplyProfileTargetAction: - intent.Kind = app.ApplyProfileToTarget - case app.ReapplyActiveProfileAction: - intent.Kind = app.ReapplyActiveProfile - case app.RestoreOriginalAction: - intent.Kind = app.RestoreOriginal - default: - return app.Intent{}, false - } - return intent, true -} - func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { if m.operation.confirmation != nil { switch key.String() { @@ -325,7 +306,7 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: reason} return m, nil } - intent, _ := targetIntent(kind, selected.ID) + intent, _ := kind.Intent(selected.ID) m.driftResolutionTargetID = "" return m, m.startOperation(intent) } @@ -423,7 +404,7 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { m.driftResolutionTargetID = selected.ID break } - intent, ok := targetIntent(selected.PrimaryAction.Kind, selected.ID) + intent, ok := selected.PrimaryAction.Kind.Intent(selected.ID) if !ok { m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: "This Target action is unavailable"} break diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index 968b227..731bcf1 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -11,7 +11,6 @@ import ( "proxyctl/internal/app" "proxyctl/internal/probe" "proxyctl/internal/profile" - "proxyctl/internal/target" ) type recordingModule struct { @@ -19,43 +18,10 @@ type recordingModule struct { intents []app.Intent } -func withTestTargetActions(snapshot app.Snapshot) app.Snapshot { - for i := range snapshot.Targets { - item := &snapshot.Targets[i] - if item.PrimaryAction.Kind != 0 { - continue - } - switch item.State { - case target.Disabled: - item.PrimaryAction.Kind = app.EnableTargetAction - case target.Enabled: - item.PrimaryAction.Kind = app.DisableTargetAction - case target.External: - item.PrimaryAction.Kind = app.ApplyProfileTargetAction - case target.Drifted: - item.PrimaryAction.Kind = app.ResolveDriftTargetAction - item.DriftResolutionActions = []app.TargetAction{{Kind: app.ReapplyActiveProfileAction, Available: snapshot.ActiveProfile != ""}, {Kind: app.RestoreOriginalAction, Available: true}} - } - item.PrimaryAction.Available = item.Available && item.DetectionPhase == app.TargetReady - if item.PrimaryAction.Available && (item.State == target.Disabled || item.State == target.External) && snapshot.ActiveProfile == "" { - item.PrimaryAction.Available = false - item.PrimaryAction.DisabledReason = "Target mutation is disabled because there is no Active Profile; open Profile management first" - } - if item.DetectionPhase == app.TargetChecking { - item.PrimaryAction.DisabledReason = "Target mutation is disabled while this Target is still being checked" - } else if item.DetectionPhase == app.TargetDetectionFailed { - item.PrimaryAction.DisabledReason = "Target mutation is disabled because this Target check failed; refresh Targets to retry" - } else if !item.Available { - item.PrimaryAction.DisabledReason = "Target mutation is disabled because this Target is unavailable" - } - } - return snapshot -} - -func (m *recordingModule) Snapshot() app.Snapshot { return withTestTargetActions(m.snapshot) } +func (m *recordingModule) Snapshot() app.Snapshot { return m.snapshot } func (m *recordingModule) Dispatch(intent app.Intent) app.Result { m.intents = append(m.intents, intent) - return app.Result{Snapshot: withTestTargetActions(m.snapshot), Notice: "done"} + return app.Result{Snapshot: m.snapshot, Notice: "done"} } func (m *recordingModule) StartOperation(intent app.Intent) app.OperationStream { result := m.Dispatch(intent) diff --git a/internal/tui/operation_coordination_test.go b/internal/tui/operation_coordination_test.go index 69c066a..8f9fbe2 100644 --- a/internal/tui/operation_coordination_test.go +++ b/internal/tui/operation_coordination_test.go @@ -18,7 +18,7 @@ type interactiveScriptModule struct { intents []app.Intent } -func (m *interactiveScriptModule) Snapshot() app.Snapshot { return withTestTargetActions(m.snapshot) } +func (m *interactiveScriptModule) Snapshot() app.Snapshot { return m.snapshot } func (m *interactiveScriptModule) Dispatch(intent app.Intent) app.Result { m.intents = append(m.intents, intent) return app.Result{Snapshot: m.snapshot} @@ -27,18 +27,18 @@ func (m *interactiveScriptModule) StartOperation(intent app.Intent) app.Operatio m.intents = append(m.intents, intent) stream := m.streams[0] m.streams = m.streams[1:] - return app.OperationStream{Snapshot: withTestTargetActions(m.snapshot), Updates: stream} + return app.OperationStream{Snapshot: m.snapshot, Updates: stream} } func operationSnapshot() app.Snapshot { - return withTestTargetActions(app.Snapshot{ + return app.Snapshot{ ActiveProfile: "office", Profiles: []profile.Profile{{Name: "office"}, {Name: "remote"}}, Targets: []app.TargetSnapshot{ - {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell details"}, Available: true, State: target.Disabled}, - {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT details"}, Available: true, State: target.Enabled}, + {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell details"}, Available: true, State: target.Disabled, PrimaryAction: app.TargetAction{Kind: app.EnableTargetAction, Available: true}}, + {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT details"}, Available: true, State: target.Enabled, PrimaryAction: app.TargetAction{Kind: app.DisableTargetAction, Available: true}}, }, - }) + } } func applyOperationCommand(t *testing.T, model Model, command tea.Cmd) (Model, tea.Cmd) { @@ -196,13 +196,15 @@ func TestCompleteModelExplainsDisabledTargetMutations(t *testing.T) { model.snapshot.Targets[0].DetectionPhase = app.TargetChecking model.snapshot.Targets[0].PrimaryAction.Available = false model.snapshot.Targets[0].PrimaryAction.DisabledReason = "Target mutation is disabled while this Target is still being checked" + model.operation.active = true updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - if command != nil || !strings.Contains(model.View(), "disabled while this Target is still being checked") { - t.Fatalf("Checking mutation lacked a visible reason:\n%s", model.View()) + if command != nil || model.operation.status.Summary != "Target mutation is disabled while this Target is still being checked" { + t.Fatalf("Checking reason was masked by operation serialization: %q\n%s", model.operation.status.Summary, model.View()) } + model.operation.active = false model.snapshot.Targets[0].DetectionPhase = app.TargetReady model.snapshot.Targets[0].PrimaryAction.Available = false model.snapshot.Targets[0].PrimaryAction.DisabledReason = "Target mutation is disabled because there is no Active Profile; open Profile management first" diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index ec3cb4e..a4e9c66 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -18,10 +18,9 @@ type targetModule struct { intents []app.Intent } -func (m *targetModule) Snapshot() app.Snapshot { return withTestTargetActions(m.snapshot) } +func (m *targetModule) Snapshot() app.Snapshot { return m.snapshot } func (m *targetModule) Dispatch(intent app.Intent) app.Result { m.intents = append(m.intents, intent) - m.result.Snapshot = withTestTargetActions(m.result.Snapshot) m.snapshot = m.result.Snapshot return m.result } @@ -44,10 +43,10 @@ func (m *targetModule) StartOperation(intent app.Intent) app.OperationStream { func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing.T) { module := &targetModule{snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{ - {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Exports for Bash/Zsh"}, Available: true, State: target.External, Evidence: "existing proxy"}, + {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Exports for Bash/Zsh"}, Available: true, State: target.External, Evidence: "existing proxy", PrimaryAction: app.TargetAction{Kind: app.ApplyProfileTargetAction, Available: true}}, {ID: "apt", Description: target.Description{Name: "APT"}, Available: false}, }}} - module.result = app.Result{Snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell", Summary: "Exports for Bash/Zsh"}, Available: true, State: target.Enabled, Evidence: "matches"}}}, Notice: "Shell Target is now Enabled"} + module.result = app.Result{Snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell", Summary: "Exports for Bash/Zsh"}, Available: true, State: target.Enabled, Evidence: "matches", PrimaryAction: app.TargetAction{Kind: app.DisableTargetAction, Available: true}}}}, Notice: "Shell Target is now Enabled"} model := NewModel(module) view := model.View() if !strings.Contains(view, "Shell") || strings.Contains(view, "APT") || !strings.Contains(view, "Action: Apply Profile") { @@ -68,7 +67,7 @@ func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing } func TestTargetRestartConfirmationCancelsWithoutConfirmedMutation(t *testing.T) { - initial := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "docker-daemon", Description: target.Description{Name: "Docker daemon"}, Available: true, State: target.Disabled}}} + initial := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "docker-daemon", Description: target.Description{Name: "Docker daemon"}, Available: true, State: target.Disabled, PrimaryAction: app.TargetAction{Kind: app.EnableTargetAction, Available: true}}}} module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, ConfirmationRequest: &app.Confirmation{Title: "Confirm Docker restart", Summary: "Docker will restart immediately and may disrupt running work.", ConfirmLabel: "Continue", CancelLabel: "Cancel"}}} model := NewModel(module) updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) @@ -89,7 +88,7 @@ func TestTargetRestartConfirmationCancelsWithoutConfirmedMutation(t *testing.T) updated, _ = model.Update(command()) model = updated.(Model) module.result = app.Result{ - Snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "docker-daemon", Description: target.Description{Name: "Docker daemon"}, Available: true, State: target.Enabled}}}, + Snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "docker-daemon", Description: target.Description{Name: "Docker daemon"}, Available: true, State: target.Enabled, PrimaryAction: app.TargetAction{Kind: app.DisableTargetAction, Available: true}}}}, Notice: "Docker daemon Target is now Enabled", } updated, command = model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'y'}}) @@ -105,7 +104,7 @@ func TestTargetRestartConfirmationCancelsWithoutConfirmedMutation(t *testing.T) func TestHelpListsEveryTargetAndMainListsOnlyAvailableTargets(t *testing.T) { module := &targetModule{snapshot: app.Snapshot{Targets: []app.TargetSnapshot{ - {ID: "shell", Description: target.Description{Name: "Shell", Summary: "summary", Details: "affects future Shell commands; does not affect daemons; owns a generated fragment; user permission; reload side effect"}, Available: true, State: target.Disabled, Evidence: "integration installed"}, + {ID: "shell", Description: target.Description{Name: "Shell", Summary: "summary", Details: "affects future Shell commands; does not affect daemons; owns a generated fragment; user permission; reload side effect"}, Available: true, State: target.Disabled, Evidence: "integration installed", PrimaryAction: app.TargetAction{Kind: app.EnableTargetAction, Available: false, DisabledReason: "This action is unavailable because there is no Active Profile; open Profile management first"}}, {ID: "apt", Description: target.Description{Name: "APT", Summary: "persistent package proxy", Details: "owns an overlay and requires system write permission"}, Available: false, AvailabilityEvidence: "Debian APT tool apt-config was not detected"}, }}} model := NewModel(module) @@ -348,7 +347,11 @@ func TestTargetPanelRendersEveryTargetState(t *testing.T) { } func TestDriftedTargetWithoutActiveProfileRequiresExplicitResolutionDirection(t *testing.T) { - initial := withTestTargetActions(app.Snapshot{Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Drifted}}}) + initial := app.Snapshot{Targets: []app.TargetSnapshot{{ + ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Drifted, + PrimaryAction: app.TargetAction{Kind: app.ResolveDriftTargetAction, Available: true}, + DriftResolutionActions: []app.TargetAction{{Kind: app.ReapplyActiveProfileAction}, {Kind: app.RestoreOriginalAction, Available: true}}, + }}} initial.Targets[0].DriftResolutionActions[0].DisabledReason = "Reapply Active Profile is unavailable because there is no Active Profile" module := &targetModule{snapshot: initial} model := NewModel(module) @@ -360,7 +363,10 @@ func TestDriftedTargetWithoutActiveProfileRequiresExplicitResolutionDirection(t } func TestTargetFailurePreservesTruthfulStateAndRendersRedactedError(t *testing.T) { - initial := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}} + initial := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ + ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled, + PrimaryAction: app.TargetAction{Kind: app.DisableTargetAction, Available: true}, + }}} module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, Err: errors.New("permission denied for alice:REDACTED@proxy")}} model := NewModel(module) updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace})