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 162f8d8..e429b45 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ proxyctl Press `m` for Proxy Profile management, then `n` to create a Profile, enter a name and proxy URL, and press Ctrl+S to save. The management page supports reviewing obscured connection details, editing, and safe deletion; Active Profile switching remains a separate dashboard picker workflow rather than a management action. 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. -The dashboard shows the Active Profile. Press `p` to open the searchable Active Profile picker, type to filter, use arrows to select, and press Enter to activate; Escape cancels without changing 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` to leave. A Target cannot be enabled without an Active Profile, but previously managed configuration can still be disabled. +The dashboard shows the Active Profile. Press `p` to open the searchable Active Profile picker, type to filter, use arrows to select, and press Enter to activate; Escape cancels without changing 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 2eb0456..156534d 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -19,8 +19,13 @@ const ( UpdateProfile DeleteProfile ActivateProfile - SetTargetDesired + SetTargetDesired // retained for synchronous compatibility; interactive callers use the explicit intents below. RefreshTargets + EnableTarget + DisableTarget + ApplyProfileToTarget + ReapplyActiveProfile + RestoreOriginal ) type Intent struct { @@ -32,6 +37,64 @@ type Intent struct { Enabled bool } +// TargetActionKind identifies a Target action independently of display labels. +type TargetActionKind uint8 + +const ( + EnableTargetAction TargetActionKind = iota + 1 + DisableTargetAction + ApplyProfileTargetAction + ResolveDriftTargetAction + ReapplyActiveProfileAction + RestoreOriginalAction +) + +// TargetAction exposes whether an intent is currently available and, when it +// is not, the reason presentation layers should show instead of ignoring it. +type TargetAction struct { + Kind TargetActionKind + Available bool + 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 ( @@ -43,14 +106,16 @@ const ( ) type TargetSnapshot struct { - ID string - Description target.Description - DetectionPhase TargetDetectionPhase - DetectionErr error - Available bool - AvailabilityEvidence string - State target.State - Evidence string + ID string + Description target.Description + DetectionPhase TargetDetectionPhase + DetectionErr error + Available bool + AvailabilityEvidence string + State target.State + Evidence string + PrimaryAction TargetAction + DriftResolutionActions []TargetAction } type Snapshot struct { @@ -154,14 +219,77 @@ func (a *Application) snapshotLocked() Snapshot { for i, id := range a.targetIDs { availability := a.availabilities[id] inspection := a.inspections[id] + phase := a.detectionPhase[id] + primary, resolution := contextualTargetActions(phase, availability.Available, inspection.State, state.ActiveProfile != "") snapshot.Targets = append(snapshot.Targets, TargetSnapshot{ - ID: id, Description: a.targetDescriptions[i], DetectionPhase: a.detectionPhase[id], DetectionErr: a.detectionErr[id], + ID: id, Description: a.targetDescriptions[i], DetectionPhase: phase, DetectionErr: a.detectionErr[id], Available: availability.Available, AvailabilityEvidence: availability.Evidence, State: inspection.State, Evidence: inspection.Evidence, + PrimaryAction: primary, DriftResolutionActions: resolution, }) } return snapshot } +func contextualTargetActions(phase TargetDetectionPhase, available bool, state target.State, hasActiveProfile bool) (TargetAction, []TargetAction) { + 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 + } + 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" + } + if state != target.Drifted { + return primary, nil + } + + 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 { + for _, policy := range targetActionPolicies { + if policy.primary && policy.state == state { + return policy.kind + } + } + return 0 +} + +func targetActionDisabledReason(phase TargetDetectionPhase, available bool) string { + switch phase { + case TargetChecking: + return "Target actions are unavailable while this Target is still being checked" + case TargetDetectionFailed: + return "Target actions are unavailable because this Target check failed; refresh Targets to retry" + default: + if !available { + return "Target actions are unavailable because this Target is unavailable" + } + return "" + } +} + type targetDetectionOutcome struct { targetID string detection target.Detection @@ -261,10 +389,10 @@ 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 intent.Kind == SetTargetDesired || intent.Kind == ActivateProfile { + if intent.Kind == ActivateProfile { a.refreshInspections() } - if intent.Kind == SetTargetDesired { + if isTargetMutationIntent(intent.Kind) { return a.setTargetDesired(intent, publish) } if intent.Kind == RefreshTargets { @@ -493,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() @@ -580,6 +725,69 @@ func (a *Application) refreshInspections() { a.replaceInspections(inspections) } +func isTargetMutationIntent(kind IntentKind) bool { + if kind == SetTargetDesired { + return true + } + for _, policy := range targetActionPolicies { + if policy.intent == kind { + return true + } + } + return 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 { selectedIndex := -1 for i, id := range a.targetIDs { @@ -593,20 +801,27 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu } selected := a.targets[selectedIndex] active := a.activeProfile() - if intent.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(intent.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(), intent.Enabled, active) + result := selected.SetDesired(context.Background(), policy.desired, active) inspection := result.Inspection if result.Err != nil { inspection = selected.Inspect(context.Background(), active) @@ -617,11 +832,7 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu outcome, detail = "failure", result.Err.Error() } if a.logger != nil { - operation := "disable" - if intent.Enabled { - operation = "enable" - } - _ = 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 diff --git a/internal/app/contextual_actions_test.go b/internal/app/contextual_actions_test.go new file mode 100644 index 0000000..a21f08b --- /dev/null +++ b/internal/app/contextual_actions_test.go @@ -0,0 +1,373 @@ +package app_test + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" + + "proxyctl/internal/app" + "proxyctl/internal/profile" + "proxyctl/internal/target" +) + +func TestApplicationSnapshotStructuresContextualTargetActions(t *testing.T) { + states := []struct { + id string + state target.State + action app.TargetActionKind + }{ + {id: "disabled", state: target.Disabled, action: app.EnableTargetAction}, + {id: "enabled", state: target.Enabled, action: app.DisableTargetAction}, + {id: "external", state: target.External, action: app.ApplyProfileTargetAction}, + {id: "drifted", state: target.Drifted, action: app.ResolveDriftTargetAction}, + } + adapters := make([]target.Target, 0, len(states)) + for _, item := range states { + adapters = append(adapters, targetWithID{fakeTarget: &fakeTarget{available: true, state: item.state}, id: item.id}) + } + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), adapters) + 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) + } + + snapshot := application.Snapshot() + for i, want := range states { + got := snapshot.Targets[i] + if got.PrimaryAction.Kind != want.action || !got.PrimaryAction.Available || got.PrimaryAction.DisabledReason != "" { + t.Errorf("%s primary action = %#v, want available %v", want.state, got.PrimaryAction, want.action) + } + } + drift := snapshot.Targets[3] + if len(drift.DriftResolutionActions) != 2 || drift.DriftResolutionActions[0].Kind != app.ReapplyActiveProfileAction || drift.DriftResolutionActions[1].Kind != app.RestoreOriginalAction { + t.Fatalf("Drift Resolution actions = %#v", drift.DriftResolutionActions) + } + for _, action := range drift.DriftResolutionActions { + if !action.Available || action.DisabledReason != "" { + t.Errorf("Drift Resolution action = %#v, want available", action) + } + } +} + +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 + state target.State + intent app.IntentKind + enabled bool + }{ + {name: "Enable", state: target.Disabled, intent: app.EnableTarget, enabled: true}, + {name: "Disable", state: target.Enabled, intent: app.DisableTarget, enabled: false}, + {name: "Apply Profile", state: target.External, intent: app.ApplyProfileToTarget, enabled: true}, + {name: "Reapply Active Profile", state: target.Drifted, intent: app.ReapplyActiveProfile, enabled: true}, + {name: "Restore Original", state: target.Drifted, intent: app.RestoreOriginal, enabled: false}, + } + for _, test := range tests { + t.Run(test.name, 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{adapter}) + if err != nil { + t.Fatal(err) + } + createAndActivate(t, application) + adapter.state = test.state + adapter.calls = nil + 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 || len(adapter.calls) != 1 || adapter.calls[0] != test.enabled { + t.Fatalf("result = %#v, SetDesired calls = %#v", result, adapter.calls) + } + wantState := target.Disabled + if test.enabled { + wantState = target.Enabled + } + if result.Snapshot.Targets[0].State != wantState { + t.Fatalf("final state = %s, want %s", result.Snapshot.Targets[0].State, wantState) + } + }) + } +} + +func TestApplicationRejectsContextualIntentWhenTargetStateChanged(t *testing.T) { + adapter := &fakeTarget{available: true, state: target.External} + 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.EnableTarget, TargetID: adapter.ID()}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "External") || len(adapter.calls) != 0 { + t.Fatalf("stale Enable intent = %#v, calls = %#v", result, adapter.calls) + } +} + +func TestApplicationReapplyPublishesTruthfulDriftWhenAdapterCannotConverge(t *testing.T) { + adapter := &fakeTarget{available: true, state: target.Disabled} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) + if err != nil { + t.Fatal(err) + } + createAndActivate(t, application) + adapter.state = target.Drifted + adapter.setState = target.Drifted + adapter.calls = nil + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } + + result := application.Dispatch(app.Intent{Kind: app.ReapplyActiveProfile, TargetID: adapter.ID()}) + if result.Err != nil || result.Snapshot.Targets[0].State != target.Drifted || result.Progress[0].State != target.Drifted { + t.Fatalf("truthful non-converged reapply = %#v", result) + } +} + +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}) + if err != nil { + t.Fatal(err) + } + createAndActivate(t, application) + adapter.calls = nil + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } + if action := application.Snapshot().Targets[0].PrimaryAction; action.Available || !strings.Contains(action.DisabledReason, "unavailable") { + t.Fatalf("unavailable action = %#v", action) + } + + result := application.Dispatch(app.Intent{Kind: app.ApplyProfileToTarget, TargetID: adapter.ID()}) + if result.Err == nil || !strings.Contains(result.Err.Error(), "unavailable") || len(adapter.calls) != 0 { + t.Fatalf("unavailable action result = %#v, calls=%#v", result, adapter.calls) + } +} + +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) + } + + // 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) + } +} + +func TestApplicationReapplyAndRestoreFailuresPublishFreshDriftedState(t *testing.T) { + for _, test := range []struct { + name string + intent app.IntentKind + err string + }{ + {name: "reapply", intent: app.ReapplyActiveProfile, err: "write failed"}, + {name: "restore ownership loss", intent: app.RestoreOriginal, err: "restoration evidence is insufficient; refusing to overwrite external changes"}, + } { + t.Run(test.name, func(t *testing.T) { + adapter := &fakeTarget{available: true, state: target.Drifted, setErr: errors.New(test.err)} + 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: test.intent, TargetID: adapter.ID()}) + if result.Err == nil || !strings.Contains(result.Err.Error(), test.err) || result.Snapshot.Targets[0].State != target.Drifted || result.Snapshot.Targets[0].Evidence != "actual state" { + t.Fatalf("failure = %#v", result) + } + if len(adapter.calls) != 1 || adapter.calls[0] != (test.intent == app.ReapplyActiveProfile) { + t.Fatalf("SetDesired calls = %#v", adapter.calls) + } + }) + } +} + +func TestApplicationSnapshotDisablesProfileDependentActionsWithoutActiveProfile(t *testing.T) { + adapters := []target.Target{ + targetWithID{fakeTarget: &fakeTarget{available: true, state: target.Disabled}, id: "disabled"}, + targetWithID{fakeTarget: &fakeTarget{available: true, state: target.External}, id: "external"}, + targetWithID{fakeTarget: &fakeTarget{available: true, state: target.Drifted}, id: "drifted"}, + } + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), adapters) + if err != nil { + t.Fatal(err) + } + if result := application.Dispatch(app.Intent{Kind: app.RefreshTargets}); result.Err != nil { + t.Fatal(result.Err) + } + + snapshot := application.Snapshot() + for _, index := range []int{0, 1} { + action := snapshot.Targets[index].PrimaryAction + if action.Available || !strings.Contains(action.DisabledReason, "no Active Profile") { + t.Errorf("profile-dependent primary action = %#v", action) + } + } + drift := snapshot.Targets[2] + if !drift.PrimaryAction.Available { + t.Fatalf("Restore Original should keep Resolve available: %#v", drift.PrimaryAction) + } + if got := drift.DriftResolutionActions[0]; got.Available || !strings.Contains(got.DisabledReason, "no Active Profile") { + t.Fatalf("Reapply action without Active Profile = %#v", got) + } + if got := drift.DriftResolutionActions[1]; !got.Available { + t.Fatalf("Restore Original action without Active Profile = %#v", got) + } +} 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 155dcab..84185d1 100644 --- a/internal/app/target_test.go +++ b/internal/app/target_test.go @@ -16,6 +16,7 @@ type fakeTarget struct { available bool state target.State setErr error + setState target.State confirmation string calls []bool } @@ -34,7 +35,9 @@ func (f *fakeTarget) Confirmation(bool) string { return f.confirmation } func (f *fakeTarget) SetDesired(_ context.Context, enabled bool, _ *profile.Profile) target.OperationResult { f.calls = append(f.calls, enabled) if f.setErr == nil { - if enabled { + if f.setState != "" { + f.state = f.setState + } else if enabled { f.state = target.Enabled } else { f.state = target.Disabled @@ -83,12 +86,13 @@ func TestApplicationRequiresTargetConfirmationBeforeMutation(t *testing.T) { t.Fatal(err) } createAndActivate(t, application) + refreshApplicationTargets(t, application) - warning := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "docker-daemon", Enabled: true}) + 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 { t.Fatalf("unconfirmed result = %#v, calls=%v", warning, adapter.calls) } - confirmed := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "docker-daemon", Enabled: true, Confirm: true}) + confirmed := application.Dispatch(app.Intent{Kind: app.EnableTarget, TargetID: "docker-daemon", Confirm: true}) if confirmed.Err != nil || len(adapter.calls) != 1 || confirmed.Snapshot.Targets[0].State != target.Enabled { t.Fatalf("confirmed result = %#v, calls=%v", confirmed, adapter.calls) } @@ -116,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 new file mode 100644 index 0000000..7d74fe8 --- /dev/null +++ b/internal/tui/contextual_actions_test.go @@ -0,0 +1,170 @@ +package tui + +import ( + "errors" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "proxyctl/internal/app" + "proxyctl/internal/target" +) + +func completeTargetSnapshot(state target.State, primary app.TargetAction) app.Snapshot { + return app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ + ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: state, PrimaryAction: primary, + }}} +} + +func TestCompleteModelRendersAndDispatchesContextualPrimaryActions(t *testing.T) { + tests := []struct { + name string + state target.State + action app.TargetActionKind + intent app.IntentKind + label string + }{ + {name: "Disabled", state: target.Disabled, action: app.EnableTargetAction, intent: app.EnableTarget, label: "Enable"}, + {name: "Enabled", state: target.Enabled, action: app.DisableTargetAction, intent: app.DisableTarget, label: "Disable"}, + {name: "External", state: target.External, action: app.ApplyProfileTargetAction, intent: app.ApplyProfileToTarget, label: "Apply Profile"}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + initial := completeTargetSnapshot(test.state, app.TargetAction{Kind: test.action, Available: true}) + module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, Notice: "done"}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + view := model.View() + if !strings.Contains(view, "Action: "+test.label) || !strings.Contains(view, "Space: "+test.label) { + t.Fatalf("contextual action not rendered:\n%s", view) + } + if test.state == target.External && (!strings.Contains(view, "proxyctl will begin managing this configuration") || !strings.Contains(view, "while preserving restorable state")) { + t.Fatalf("Apply Profile ownership explanation missing:\n%s", view) + } + + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if command == nil || len(module.intents) != 1 || module.intents[0].Kind != test.intent { + t.Fatalf("intent = %#v, command=%v", module.intents, command != nil) + } + }) + } +} + +func TestCompleteModelOffersBothExplicitDriftResolutionIntents(t *testing.T) { + initial := completeTargetSnapshot(target.Drifted, app.TargetAction{Kind: app.ResolveDriftTargetAction, Available: true}) + initial.Targets[0].DriftResolutionActions = []app.TargetAction{ + {Kind: app.ReapplyActiveProfileAction, Available: true}, + {Kind: app.RestoreOriginalAction, Available: true}, + } + for _, test := range []struct { + key string + intent app.IntentKind + }{ + {key: "r", intent: app.ReapplyActiveProfile}, + {key: "o", intent: app.RestoreOriginal}, + } { + t.Run(test.key, func(t *testing.T) { + module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, Notice: "done"}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + model = updateKey(model, " ") + view := model.View() + for _, want := range []string{"Drift Resolution", "Reapply Active Profile", "Restore Original"} { + if !strings.Contains(view, want) { + t.Fatalf("missing %q:\n%s", want, view) + } + } + updated, command := model.Update(keyMessage(test.key)) + model = updated.(Model) + if command == nil || len(module.intents) != 1 || module.intents[0].Kind != test.intent { + t.Fatalf("resolution intent = %#v, command=%v", module.intents, command != nil) + } + }) + } +} + +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", + }) + module := &targetModule{snapshot: initial} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + view := model.View() + if !strings.Contains(view, "No Active Profile is selected") || !strings.Contains(view, "Space: Enable disabled") { + t.Fatalf("structured disabled reason missing:\n%s", view) + } + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if command != nil || len(module.intents) != 0 || !strings.Contains(model.View(), "No Active Profile is selected") { + t.Fatalf("disabled action mutated: intents=%#v\n%s", module.intents, model.View()) + } +} + +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{ + {Kind: app.ReapplyActiveProfileAction, Available: true}, + {Kind: app.RestoreOriginalAction, Available: true}, + } + module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, Err: errors.New("restoration evidence is insufficient; external changes were not overwritten")}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + model = updateKey(model, " ") + updated, command := model.Update(keyMessage("o")) + model = updated.(Model) + if command == nil { + t.Fatal("Restore Original did not start") + } + updated, _ = model.Update(command()) + model = updated.(Model) + if len(module.intents) != 1 || module.intents[0].Kind != app.RestoreOriginal || !strings.Contains(model.View(), "Current error") || !strings.Contains(model.View(), "Drifted") { + t.Fatalf("ownership-loss result: intents=%#v\n%s", module.intents, model.View()) + } +} + +func TestCompleteModelCancelsDriftResolutionWithoutMutation(t *testing.T) { + initial := completeTargetSnapshot(target.Drifted, app.TargetAction{Kind: app.ResolveDriftTargetAction, Available: true}) + initial.Targets[0].DriftResolutionActions = []app.TargetAction{{Kind: app.ReapplyActiveProfileAction, Available: true}, {Kind: app.RestoreOriginalAction, Available: true}} + module := &targetModule{snapshot: initial} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + model = updateKey(model, " ") + updated, command := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) + model = updated.(Model) + if command != nil || len(module.intents) != 0 || strings.Contains(model.View(), "r: Reapply Active Profile") { + t.Fatalf("safe-default cancellation mutated: intents=%#v\n%s", module.intents, model.View()) + } +} diff --git a/internal/tui/dashboard.go b/internal/tui/dashboard.go index f88942b..888d774 100644 --- a/internal/tui/dashboard.go +++ b/internal/tui/dashboard.go @@ -5,6 +5,25 @@ import ( "proxyctl/internal/target" ) +func targetActionLabel(kind app.TargetActionKind) string { + switch kind { + case app.EnableTargetAction: + return "Enable" + case app.DisableTargetAction: + return "Disable" + case app.ApplyProfileTargetAction: + return "Apply Profile" + case app.ResolveDriftTargetAction: + return "Resolve" + case app.ReapplyActiveProfileAction: + return "Reapply Active Profile" + case app.RestoreOriginalAction: + return "Restore Original" + default: + return "Unavailable" + } +} + type dashboardPane uint8 const ( diff --git a/internal/tui/dashboard_view.go b/internal/tui/dashboard_view.go index 8b55ccb..9cd89b0 100644 --- a/internal/tui/dashboard_view.go +++ b/internal/tui/dashboard_view.go @@ -142,6 +142,16 @@ 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) + 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 dbcc3a3..2d94763 100644 --- a/internal/tui/foundation_test.go +++ b/internal/tui/foundation_test.go @@ -51,11 +51,11 @@ func TestCompleteModelResizePreservesValidSelectionAndFormFocus(t *testing.T) { model := resizedModel(t, module, themeModeNoColor, 120, 36) model = updateKey(model, "down") model = resize(model, 80, 24) - if view := model.View(); !strings.Contains(view, "> ◇ External APT") || !strings.Contains(view, "APT summary") { + if view := model.View(); !strings.Contains(view, "> ◇ External APT") || !strings.Contains(view, "Action: Apply Profile") { t.Fatalf("Target selection did not survive wide-to-narrow resize:\n%s", view) } model = resize(model, 120, 36) - if view := model.View(); !strings.Contains(view, "> ◇ External APT") || !strings.Contains(view, "APT summary") { + if view := model.View(); !strings.Contains(view, "> ◇ External APT") || !strings.Contains(view, "Action: Apply Profile") { t.Fatalf("Target selection did not survive narrow-to-wide resize:\n%s", view) } @@ -199,7 +199,7 @@ func TestCompleteModelNarrowShellKeepsLastProductionTargetStatusAndActionsVisibl "State: ! Drifted", "Docker daemon selected evidence.", "Status: Refresh completed; selected Target remains actionable.", - "Space: toggle", + "Space: Resolve", "p: switch Active Profile", "m: manage Proxy Profiles", ) @@ -210,7 +210,7 @@ func TestCompleteModelNarrowShellKeepsLastProductionTargetStatusAndActionsVisibl "> ! Drifted Docker daemon", "State: ! Drifted", "Docker daemon selected evidence.", - "Space: toggle", + "Space: Resolve", ) } @@ -247,11 +247,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}}}, }, } } @@ -263,8 +263,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 c436308..b3fce60 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -11,7 +11,6 @@ import ( "proxyctl/internal/app" "proxyctl/internal/probe" "proxyctl/internal/profile" - "proxyctl/internal/target" ) type page int @@ -32,26 +31,27 @@ type formState struct { } type Model struct { - application app.InteractiveModule - presentation terminalPresentation - snapshot app.Snapshot - page page - width int - height int - selected int - selectedProfileName string - profileViewportOffset int - profileDetailsOffset int - profileDelete *profileDeleteConfirmation - selectedTargetID string - dashboardFocus dashboardPane - showUnavailable bool - detailsOffset int - picker profilePickerState - form formState - status string - operation operationCoordinator - detect func(context.Context, string) (probe.Capabilities, error) + application app.InteractiveModule + presentation terminalPresentation + snapshot app.Snapshot + page page + width int + height int + selected int + selectedProfileName string + profileViewportOffset int + profileDetailsOffset int + profileDelete *profileDeleteConfirmation + selectedTargetID string + dashboardFocus dashboardPane + showUnavailable bool + detailsOffset int + driftResolutionTargetID string + picker profilePickerState + form formState + status string + operation operationCoordinator + detect func(context.Context, string) (probe.Capabilities, error) } // probeDone carries one finished Protocol Probe back to the form. address is @@ -252,27 +252,35 @@ type mutationAvailability struct { compactReason string } -// targetMutationAvailability is the sole policy used by both input handling -// and rendering, so the displayed disabled reason cannot drift from behavior. -func targetMutationAvailability(selected app.TargetSnapshot, selectedOK bool, activeProfile string, operationBlocked bool) mutationAvailability { +// targetMutationAvailability consumes the Application's structured action +// policy. It adds only Model-owned serialization state and never derives an +// 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.DetectionPhase == app.TargetChecking: - return mutationAvailability{reason: "Target mutation is disabled while this Target is still being checked", compactReason: "checking Target"} - case selected.DetectionPhase == app.TargetDetectionFailed: - return mutationAvailability{reason: "Target mutation is disabled because this Target check failed; refresh Targets to retry", compactReason: "Target check failed"} - case !selected.Available: - return mutationAvailability{reason: "Target mutation is disabled because this Target is unavailable", compactReason: "Target unavailable"} - case (selected.State == target.Disabled || selected.State == target.External) && activeProfile == "": - return mutationAvailability{reason: "Target mutation is disabled because there is no Active Profile; open Profile management first", compactReason: "no Active Profile"} + case !selected.PrimaryAction.Available: + reason := selected.PrimaryAction.DisabledReason + if reason == "" { + 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} } } +func driftResolutionAction(selected app.TargetSnapshot, kind app.TargetActionKind) (app.TargetAction, bool) { + for _, action := range selected.DriftResolutionActions { + if action.Kind == kind { + return action, true + } + } + return app.TargetAction{Kind: kind}, false +} + func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { if m.picker.open { switch key.String() { @@ -304,6 +312,37 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { } return m, nil } + if m.driftResolutionTargetID != "" { + switch key.String() { + case "enter", "esc", "n": + m.driftResolutionTargetID = "" + return m, nil + case "r", "o": + kind := app.ReapplyActiveProfileAction + if key.String() == "o" { + kind = app.RestoreOriginalAction + } + selected, ok := newTargetCatalog(m.snapshot, true, m.driftResolutionTargetID).selected(m.driftResolutionTargetID) + if !ok { + m.driftResolutionTargetID = "" + m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Drift Resolution is unavailable because the Target is no longer present"} + return m, nil + } + action, ok := driftResolutionAction(selected, kind) + if !ok || !action.Available { + reason := action.DisabledReason + if reason == "" { + reason = "This Drift Resolution action is unavailable" + } + m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: reason} + return m, nil + } + intent, _ := kind.Intent(selected.ID) + m.driftResolutionTargetID = "" + return m, m.startOperation(intent) + } + return m, nil + } if key.String() == "e" && m.operation.currentError != nil { m.operation.errorExpanded = !m.operation.errorExpanded m.operation.errorOffset = 0 @@ -393,13 +432,20 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { } case " ": selected, ok := catalog.selected(m.selectedTargetID) - availability := targetMutationAvailability(selected, ok, m.snapshot.ActiveProfile, m.operation.active || m.operation.quitRequested) + availability := targetMutationAvailability(selected, ok, m.operation.active || m.operation.quitRequested) if !availability.available { m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: availability.reason} break } - desired := selected.State == target.Disabled || selected.State == target.External - intent := app.Intent{Kind: app.SetTargetDesired, TargetID: selected.ID, Enabled: desired} + if selected.PrimaryAction.Kind == app.ResolveDriftTargetAction { + m.driftResolutionTargetID = selected.ID + break + } + 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 + } return m, m.startOperation(intent) } @@ -662,30 +708,31 @@ func (m *Model) ensureProfileSelectionVisible() { func (m Model) View() string { return m.presentation.Render(presentationState{ - snapshot: m.snapshot, - page: m.page, - width: m.width, - height: m.height, - selected: m.selected, - profileOffset: m.profileViewportOffset, - profileDetails: m.profileDetailsOffset, - profileDelete: m.profileDelete, - selectedTargetID: m.selectedTargetID, - dashboardFocus: m.dashboardFocus, - showUnavailable: m.showUnavailable, - detailsOffset: m.detailsOffset, - picker: m.picker, - form: m.form, - confirmation: m.operation.confirmation, - confirmationDialog: &m.operation.confirmationDialog, - status: m.status, - operationStatus: m.operation.status, - currentError: m.operation.currentError, - errorExpanded: m.operation.errorExpanded, - errorOffset: m.operation.errorOffset, - exitDeferred: m.operation.quitRequested && m.operationInFlight(), - operationActive: m.operation.active, - operationProgress: m.operation.progress, + snapshot: m.snapshot, + page: m.page, + width: m.width, + height: m.height, + selected: m.selected, + profileOffset: m.profileViewportOffset, + profileDetails: m.profileDetailsOffset, + profileDelete: m.profileDelete, + selectedTargetID: m.selectedTargetID, + dashboardFocus: m.dashboardFocus, + showUnavailable: m.showUnavailable, + detailsOffset: m.detailsOffset, + driftResolutionTargetID: m.driftResolutionTargetID, + picker: m.picker, + form: m.form, + confirmation: m.operation.confirmation, + confirmationDialog: &m.operation.confirmationDialog, + status: m.status, + operationStatus: m.operation.status, + currentError: m.operation.currentError, + errorExpanded: m.operation.errorExpanded, + errorOffset: m.operation.errorOffset, + exitDeferred: m.operation.quitRequested && m.operationInFlight(), + operationActive: m.operation.active, + operationProgress: m.operation.progress, }) } diff --git a/internal/tui/operation_coordination_test.go b/internal/tui/operation_coordination_test.go index c6625f2..8f9fbe2 100644 --- a/internal/tui/operation_coordination_test.go +++ b/internal/tui/operation_coordination_test.go @@ -35,8 +35,8 @@ func operationSnapshot() 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}}, }, } } @@ -194,14 +194,20 @@ func TestCompleteModelExplainsDisabledTargetMutations(t *testing.T) { module := &interactiveScriptModule{snapshot: checking} model := resizedModel(t, module, themeModeNoColor, 120, 36) 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" updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) if command != nil || !strings.Contains(model.View(), "disabled because there is no Active Profile") { diff --git a/internal/tui/pages.go b/internal/tui/pages.go index dc34afb..d1e349a 100644 --- a/internal/tui/pages.go +++ b/internal/tui/pages.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "proxyctl/internal/app" "proxyctl/internal/profile" ) @@ -46,6 +47,13 @@ func (p terminalPresentation) mainView(model presentationState, width, height in page.primary = model.picker.view(model.snapshot, width, max(5, height-9)) page.footer = "↑/↓ PgUp/PgDn: select Enter: activate Esc: cancel" } + if model.driftResolutionTargetID != "" && model.confirmation == nil { + if selected, ok := newTargetCatalog(model.snapshot, true, model.driftResolutionTargetID).selected(model.driftResolutionTargetID); ok { + page.dashboard = nil + page.primary, page.context = p.driftResolutionView(selected) + page.footer = "r: Reapply Active Profile o: Restore Original\nEnter/Esc: Cancel" + } + } if model.confirmation != nil { dialog := newConfirmationDialog(*model.confirmation) if model.confirmationDialog != nil { @@ -58,6 +66,26 @@ func (p terminalPresentation) mainView(model presentationState, width, height in return page } +func (p terminalPresentation) driftResolutionView(selected app.TargetSnapshot) (string, string) { + var choices strings.Builder + fmt.Fprintf(&choices, "%s\nChoose how to resolve %s without an automatic recovery direction.", p.theme.section.Render("Drift Resolution — "+selected.Description.Name), selected.Description.Name) + for _, kind := range []app.TargetActionKind{app.ReapplyActiveProfileAction, app.RestoreOriginalAction} { + action, ok := driftResolutionAction(selected, kind) + label := targetActionLabel(kind) + if !ok || !action.Available { + reason := action.DisabledReason + if reason == "" { + reason = "unavailable" + } + fmt.Fprintf(&choices, "\n\n%s — unavailable: %s", label, reason) + continue + } + fmt.Fprintf(&choices, "\n\n%s", label) + } + context := "Reapply Active Profile writes the Active Profile again. Restore Original asks the Target Adapter to use its existing ownership and restoration safeguards; insufficient evidence fails safely without overwriting external changes." + return choices.String(), context +} + func (p terminalPresentation) dashboardFooter(model presentationState, catalog targetCatalog) string { if model.exitDeferred { return "Exit pending; browsing remains safe while the current mutation completes" @@ -75,11 +103,12 @@ func (p terminalPresentation) dashboardFooter(model presentationState, catalog t } actions := "↑/k ↓/j: select Tab: focus details " + visibility selected, ok := catalog.selected(model.selectedTargetID) - availability := targetMutationAvailability(selected, ok, model.snapshot.ActiveProfile, model.operationActive || model.exitDeferred) + availability := targetMutationAvailability(selected, ok, model.operationActive || model.exitDeferred) + label := targetActionLabel(selected.PrimaryAction.Kind) if availability.available { - actions = "Space: toggle " + actions + actions = "Space: " + label + " " + actions } else { - actions = "Space: disabled (" + availability.compactReason + ") " + actions + actions = "Space: " + label + " disabled (" + availability.compactReason + ") " + actions } footer := actions + "\np: switch Active Profile m: manage Proxy Profiles r: refresh ?: help q: quit" if model.operationActive { diff --git a/internal/tui/presentation.go b/internal/tui/presentation.go index f424ad7..340c41e 100644 --- a/internal/tui/presentation.go +++ b/internal/tui/presentation.go @@ -53,30 +53,31 @@ func defaultModelOptions() modelOptions { } type presentationState struct { - snapshot app.Snapshot - page page - width int - height int - selected int - profileOffset int - profileDetails int - profileDelete *profileDeleteConfirmation - selectedTargetID string - dashboardFocus dashboardPane - showUnavailable bool - detailsOffset int - picker profilePickerState - form formState - confirmation *app.Confirmation - confirmationDialog *confirmationDialog - status string - operationStatus app.OperationStatus - currentError *operationError - errorExpanded bool - errorOffset int - exitDeferred bool - operationActive bool - operationProgress map[string]app.TargetProgressPhase + snapshot app.Snapshot + page page + width int + height int + selected int + profileOffset int + profileDetails int + profileDelete *profileDeleteConfirmation + selectedTargetID string + dashboardFocus dashboardPane + showUnavailable bool + detailsOffset int + driftResolutionTargetID string + picker profilePickerState + form formState + confirmation *app.Confirmation + confirmationDialog *confirmationDialog + status string + operationStatus app.OperationStatus + currentError *operationError + errorExpanded bool + errorOffset int + exitDeferred bool + operationActive bool + operationProgress map[string]app.TargetProgressPhase } // terminalPresentation is the terminal-presentation Module. Application diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index abf2ed2..54ab469 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -43,13 +43,13 @@ 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, "Exports for Bash/Zsh") { + if !strings.Contains(view, "Shell") || strings.Contains(view, "APT") || !strings.Contains(view, "Action: Apply Profile") { t.Fatalf("Target panel:\n%s", view) } @@ -61,13 +61,13 @@ func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing message := command() updated, _ = model.Update(message) model = updated.(Model) - if len(module.intents) != 1 || module.intents[0].Kind != app.SetTargetDesired || !module.intents[0].Enabled || !strings.Contains(model.View(), "Enabled") { + if len(module.intents) != 1 || module.intents[0].Kind != app.ApplyProfileToTarget || !strings.Contains(model.View(), "Enabled") { t.Fatalf("completed toggle = %#v\n%s", module.intents, model.View()) } } 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}) @@ -88,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'}}) @@ -104,12 +104,12 @@ 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) view := model.View() - if strings.Contains(view, "APT") || !strings.Contains(view, "summary") || !strings.Contains(view, "integration installed") { + if strings.Contains(view, "APT") || !strings.Contains(view, "Action: Enable") || !strings.Contains(view, "integration installed") { t.Fatalf("main Target panel:\n%s", view) } model = updateKey(model, "?") @@ -344,22 +344,27 @@ func TestTargetPanelRendersEveryTargetState(t *testing.T) { } } -func TestDriftedTargetWithoutActiveProfileTogglesTowardDisabled(t *testing.T) { - initial := app.Snapshot{Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Drifted}}} - module := &targetModule{snapshot: initial, result: app.Result{Snapshot: app.Snapshot{Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Disabled}}}}} +func TestDriftedTargetWithoutActiveProfileRequiresExplicitResolutionDirection(t *testing.T) { + 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) - _, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) - if command == nil { - t.Fatal("Drifted Target did not start disable") - } - _ = command() - if len(module.intents) != 1 || module.intents[0].Enabled { - t.Fatalf("Drifted Target intent = %#v", module.intents) + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if command != nil || len(module.intents) != 0 || !strings.Contains(model.View(), "Drift Resolution") || !strings.Contains(model.View(), "Reapply Active Profile is unavailable") { + t.Fatalf("Drifted Target chose a direction implicitly: intents=%#v\n%s", module.intents, model.View()) } } 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})