pi-agent: Implement contextual Target actions

2026-07-17 22:37:38 +08:00
parent 7c0b4481d7
commit 2ca42bd4bd
13 changed files with 793 additions and 120 deletions
@@ -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,26 @@ 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 TargetDetectionPhase uint8

 const (
@@ -43,14 +68,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 +181,78 @@ 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) {
+	primary := TargetAction{Kind: primaryTargetActionKind(state), Available: true}
+	if reason := targetActionDisabledReason(phase, available); reason != "" {
+		primary.Available = false
+		primary.DisabledReason = reason
+	}
+	needsProfile := state == target.Disabled || state == target.External
+	if primary.Available && needsProfile && !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
+	}
+
+	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
+		}
+	}
+	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
+	}
+}
+
+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 +352,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 isTargetMutationIntent(intent.Kind) || intent.Kind == ActivateProfile {
 		a.refreshInspections()
 	}
-	if intent.Kind == SetTargetDesired {
+	if isTargetMutationIntent(intent.Kind) {
 		return a.setTargetDesired(intent, publish)
 	}
 	if intent.Kind == RefreshTargets {
@@ -580,6 +671,32 @@ func (a *Application) refreshInspections() {
 	a.replaceInspections(inspections)
 }

+func isTargetMutationIntent(kind IntentKind) bool {
+	switch kind {
+	case SetTargetDesired, EnableTarget, DisableTarget, ApplyProfileToTarget, ReapplyActiveProfile, RestoreOriginal:
+		return true
+	default:
+		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 (a *Application) setTargetDesired(intent Intent, publish func(Result)) Result {
 	selectedIndex := -1
 	for i, id := range a.targetIDs {
@@ -592,11 +709,19 @@ 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 intent.Enabled && active == nil {
+	if enabled && active == nil {
 		return Result{Snapshot: a.Snapshot(), Err: errors.New("no Active Profile; open Profile management before enabling a Target")}
 	}
-	if warning := selected.Confirmation(intent.Enabled); warning != "" && !intent.Confirm {
+	if warning := selected.Confirmation(enabled); 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,
@@ -606,7 +731,7 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu
 		publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{{TargetID: intent.TargetID, Phase: TargetProgressRunning, State: a.inspection(intent.TargetID).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(), enabled, active)
 	inspection := result.Inspection
 	if result.Err != nil {
 		inspection = selected.Inspect(context.Background(), active)
@@ -617,10 +742,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"
-		}
+		operation := targetOperationName(intent.Kind, enabled)
 		_ = a.logger.Write(LogEntry{Time: time.Now(), Operation: operation, Target: intent.TargetID, Result: outcome, Detail: RedactLogDetail(detail)})
 	}
 	notice := ""
@@ -637,6 +759,22 @@ 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 {
@@ -0,0 +1,233 @@
+package app_test
+
+import (
+	"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 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)
+	}
+}
+
+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 TestApplicationRejectsProfileDependentIntentWithoutActiveProfile(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)
+	}
+	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)
+	}
+}
+
+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)
+	}
+}
@@ -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
@@ -84,11 +87,11 @@ func TestApplicationRequiresTargetConfirmationBeforeMutation(t *testing.T) {
 	}
 	createAndActivate(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)
 	}