diff --git a/internal/app/app.go b/internal/app/app.go index cb23c63..5acf69a 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -76,12 +76,13 @@ type ProgressiveModule interface { } type Application struct { - store profile.Store - state profile.State - targets []target.Target - inspections map[string]target.Inspection - mutationLock MutationLock - logger OperationLogger + store profile.Store + state profile.State + targets []target.Target + availabilities map[string]target.Availability + inspections map[string]target.Inspection + mutationLock MutationLock + logger OperationLogger } func New(store profile.Store) (*Application, error) { @@ -105,7 +106,15 @@ func NewWithTargetsAndRuntime(store profile.Store, targets []target.Target, muta if err != nil { return nil, err } - a := &Application{store: store, state: state, targets: append([]target.Target(nil), targets...), inspections: make(map[string]target.Inspection), mutationLock: mutationLock, logger: logger} + a := &Application{ + store: store, + state: state, + targets: append([]target.Target(nil), targets...), + availabilities: make(map[string]target.Availability), + inspections: make(map[string]target.Inspection), + mutationLock: mutationLock, + logger: logger, + } a.refreshTargets() return a, nil } @@ -114,7 +123,7 @@ func (a *Application) Snapshot() Snapshot { profiles := append([]profile.Profile(nil), a.state.Profiles...) snapshot := Snapshot{Profiles: profiles, ActiveProfile: a.state.ActiveProfile} for _, adapter := range a.targets { - availability := adapter.Availability(context.Background()) + availability := a.availabilities[adapter.ID()] inspection := a.inspections[adapter.ID()] snapshot.Targets = append(snapshot.Targets, TargetSnapshot{ID: adapter.ID(), Description: adapter.Description(), Available: availability.Available, AvailabilityEvidence: availability.Evidence, State: inspection.State, Evidence: inspection.Evidence}) } @@ -171,7 +180,7 @@ func (a *Application) dispatch(intent Intent, publish func(Result)) Result { }) if err == nil { a.state = next - a.refreshTargets() + a.refreshInspections() } else { var committed *profile.CommitError if errors.As(err, &committed) { @@ -243,18 +252,22 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{{TargetID: adapter.ID(), Applying: true, State: a.inspections[adapter.ID()].State}}, Notice: "Updating " + adapter.Description().Name + " Target"}) } operation := adapter.SetDesired(context.Background(), true, active) - a.inspections[adapter.ID()] = operation.Inspection + inspection := operation.Inspection var safeErr error if operation.Err != nil { - safeErr = errors.New(profile.RedactText(operation.Err.Error())) + // A failure-returned inspection may predate a partial write. Always + // inspect explicitly against the new Active Profile before publishing. + inspection = adapter.Inspect(context.Background(), active) + safeErr = fmt.Errorf("%s (fresh inspection: %s — %s)", profile.RedactText(operation.Err.Error()), inspection.State, profile.RedactText(inspection.Evidence)) } - item := TargetProgress{TargetID: adapter.ID(), State: operation.Inspection.State, Evidence: operation.Inspection.Evidence, Err: safeErr} + a.inspections[adapter.ID()] = inspection + item := TargetProgress{TargetID: adapter.ID(), State: inspection.State, Evidence: inspection.Evidence, Err: safeErr} progress = append(progress, item) if publish != nil { publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{item}, Notice: adapter.Description().Name + " Target update completed", Err: safeErr}) } if a.logger != nil { - outcome, detail := "success", operation.Inspection.Evidence + outcome, detail := "success", inspection.Evidence if safeErr != nil { outcome, detail = "failure", safeErr.Error() } @@ -294,6 +307,14 @@ func (a *Application) activeProfile() *profile.Profile { } func (a *Application) refreshTargets() { + active := a.activeProfile() + for _, adapter := range a.targets { + a.availabilities[adapter.ID()] = adapter.Availability(context.Background()) + a.inspections[adapter.ID()] = adapter.Inspect(context.Background(), active) + } +} + +func (a *Application) refreshInspections() { active := a.activeProfile() for _, adapter := range a.targets { a.inspections[adapter.ID()] = adapter.Inspect(context.Background(), active) @@ -319,8 +340,12 @@ func (a *Application) setTargetDesired(intent Intent) Result { return Result{Snapshot: a.Snapshot(), Confirmation: warning} } result := selected.SetDesired(context.Background(), intent.Enabled, active) - a.inspections[selected.ID()] = result.Inspection - outcome, detail := "success", result.Inspection.Evidence + inspection := result.Inspection + if result.Err != nil { + inspection = selected.Inspect(context.Background(), active) + } + a.inspections[selected.ID()] = inspection + outcome, detail := "success", inspection.Evidence if result.Err != nil { outcome, detail = "failure", result.Err.Error() } @@ -333,7 +358,7 @@ func (a *Application) setTargetDesired(intent Intent) Result { } notice := "" if result.Err == nil { - notice = fmt.Sprintf("%s Target is now %s", selected.Description().Name, result.Inspection.State) + notice = fmt.Sprintf("%s Target is now %s", selected.Description().Name, inspection.State) } return Result{Snapshot: a.Snapshot(), Notice: notice, Err: result.Err} } diff --git a/internal/app/coordination_test.go b/internal/app/coordination_test.go index 561f424..5bae975 100644 --- a/internal/app/coordination_test.go +++ b/internal/app/coordination_test.go @@ -16,11 +16,13 @@ import ( ) type coordinatedTarget struct { - id string - state target.State - applied []string - appliedName string - failName string + id string + state target.State + applied []string + appliedName string + failName string + availabilityCalls int + inspectCalls int } func (t *coordinatedTarget) ID() string { return t.id } @@ -28,13 +30,15 @@ func (t *coordinatedTarget) Description() target.Description { return target.Description{Name: t.id, Summary: "summary", Details: "effects, non-effects, ownership, permissions, and side effects"} } func (t *coordinatedTarget) Availability(context.Context) target.Availability { + t.availabilityCalls++ return target.Availability{Available: true, Evidence: "detected"} } func (t *coordinatedTarget) Inspect(_ context.Context, active *profile.Profile) target.Inspection { + t.inspectCalls++ if t.state == target.Enabled && (active == nil || (t.appliedName != "" && active.Name != t.appliedName)) { return target.Inspection{State: target.Drifted, Evidence: "managed configuration does not match the Active Profile"} } - return target.Inspection{State: t.state, Evidence: "actual configuration"} + return target.Inspection{State: t.state, Evidence: "fresh actual configuration"} } func (t *coordinatedTarget) Confirmation(bool) string { return "" } func (t *coordinatedTarget) SetDesired(_ context.Context, enabled bool, active *profile.Profile) target.OperationResult { @@ -45,7 +49,7 @@ func (t *coordinatedTarget) SetDesired(_ context.Context, enabled bool, active * t.applied = append(t.applied, active.Name) if active.Name == t.failName { t.state = target.Drifted - return target.OperationResult{Inspection: target.Inspection{State: t.state, Evidence: "re-inspected partial write"}, Err: errors.New("failed using http://alice:secret@proxy.example")} + return target.OperationResult{Inspection: target.Inspection{State: target.Enabled, Evidence: "stale operation state"}, Err: errors.New("failed using http://alice:secret@proxy.example")} } t.state = target.Enabled t.appliedName = active.Name @@ -74,7 +78,15 @@ func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing. } } - result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "new"}) + inspectCallsBeforeSwitch := second.inspectCalls + var result app.Result + var failedCompletion app.Result + for update := range application.DispatchProgress(app.Intent{Kind: app.ActivateProfile, Name: "new"}) { + result = update + if len(update.Progress) == 1 && update.Progress[0].TargetID == "second" && !update.Progress[0].Applying { + failedCompletion = update + } + } if result.Snapshot.ActiveProfile != "new" { t.Fatalf("Active Profile = %q", result.Snapshot.ActiveProfile) } @@ -84,9 +96,21 @@ func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing. if got := result.Snapshot.Targets; got[0].State != target.Enabled || got[1].State != target.Drifted { t.Fatalf("truthful partial states = %#v", got) } - if len(result.Progress) != 2 || result.Progress[0].TargetID != "first" || result.Progress[0].State != target.Enabled || result.Progress[1].TargetID != "second" || result.Progress[1].State != target.Drifted { + if len(result.Progress) != 2 || result.Progress[0].TargetID != "first" || result.Progress[0].State != target.Enabled || result.Progress[1].TargetID != "second" || result.Progress[1].State != target.Drifted || result.Progress[1].Evidence != "fresh actual configuration" { t.Fatalf("per-Target progress = %#v", result.Progress) } + if len(failedCompletion.Progress) != 1 || failedCompletion.Progress[0].State != target.Drifted || failedCompletion.Progress[0].Evidence != "fresh actual configuration" || failedCompletion.Err == nil { + t.Fatalf("failed completion event was not truthful: %#v", failedCompletion) + } + if failedCompletion.Snapshot.Targets[1].State != target.Drifted || failedCompletion.Snapshot.Targets[1].Evidence != "fresh actual configuration" { + t.Fatalf("failed completion snapshot was not truthful: %#v", failedCompletion.Snapshot.Targets[1]) + } + if second.inspectCalls < inspectCallsBeforeSwitch+2 { + t.Fatalf("failed SetDesired was not explicitly re-inspected: Inspect calls before=%d after=%d", inspectCallsBeforeSwitch, second.inspectCalls) + } + if !strings.Contains(result.Err.Error(), "fresh actual configuration") || !strings.Contains(result.Err.Error(), "Drifted") { + t.Fatalf("failure summary omitted fresh inspection: %v", result.Err) + } if len(first.applied) != 2 || first.applied[1] != "new" { t.Fatalf("successful Target was not updated: %#v", first.applied) } @@ -140,6 +164,41 @@ func TestApplicationCoordinatesMultipleRealAndFakeHermeticAdaptersThroughPartial } } +func TestActiveProfileProgressReusesAvailabilityCache(t *testing.T) { + adapter := &coordinatedTarget{id: "shell", state: target.Disabled} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) + if err != nil { + t.Fatal(err) + } + for _, name := range []string{"old", "new"} { + if result := application.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{Name: name, SingleURL: "http://proxy.example:8080"}}); result.Err != nil { + t.Fatal(result.Err) + } + } + if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil { + t.Fatal(result.Err) + } + if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}); result.Err != nil { + t.Fatal(result.Err) + } + + callsBeforeSwitch := adapter.availabilityCalls + var updates int + for range application.DispatchProgress(app.Intent{Kind: app.ActivateProfile, Name: "new"}) { + updates++ + } + if updates < 3 { + t.Fatalf("progress updates = %d", updates) + } + if adapter.availabilityCalls != callsBeforeSwitch { + t.Fatalf("availability probes during progress = %d, want cached count %d", adapter.availabilityCalls, callsBeforeSwitch) + } + application.Dispatch(app.Intent{Kind: app.RefreshTargets}) + if adapter.availabilityCalls != callsBeforeSwitch+1 { + t.Fatalf("explicit refresh probe count = %d, want %d", adapter.availabilityCalls, callsBeforeSwitch+1) + } +} + func TestActiveProfileSwitchStreamsApplyingAndCompletionForEachTarget(t *testing.T) { adapter := &coordinatedTarget{id: "shell", state: target.Disabled} application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) diff --git a/internal/app/target_test.go b/internal/app/target_test.go index eeb235f..92ce6fb 100644 --- a/internal/app/target_test.go +++ b/internal/app/target_test.go @@ -68,7 +68,7 @@ func TestApplicationTargetSeamRequiresActiveProfileAndReturnsTruthfulResults(t * adapter.setErr = errors.New("permission denied") adapter.state = target.Drifted result = application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: false}) - if result.Err == nil || result.Snapshot.Targets[0].State != target.Drifted || result.Snapshot.Targets[0].Evidence != "re-inspected actual state" { + if result.Err == nil || result.Snapshot.Targets[0].State != target.Drifted || result.Snapshot.Targets[0].Evidence != "actual state" { t.Fatalf("failed operation not truthful: %#v", result) } } diff --git a/internal/target/target.go b/internal/target/target.go index cc8e218..d87d0bd 100644 --- a/internal/target/target.go +++ b/internal/target/target.go @@ -33,6 +33,10 @@ type Inspection struct { Evidence string } +// OperationResult reports the adapter's immediate operation outcome. When Err +// is non-nil, Inspection is advisory because it may have been captured before a +// partial write or command failure. The Application explicitly calls Inspect +// before publishing the final state of a failed operation. type OperationResult struct { Inspection Inspection Err error diff --git a/internal/tui/model.go b/internal/tui/model.go index 628bd45..39bcc39 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -38,6 +38,7 @@ type Model struct { pendingActivation app.Intent status string applying bool + refreshing bool profileTransition bool progressTarget string } @@ -46,6 +47,10 @@ type targetOperationDone struct { result app.Result } +type refreshDone struct { + result app.Result +} + type profileOperationProgress struct { result app.Result updates <-chan app.Result @@ -68,6 +73,19 @@ func NewModel(application app.Module) Model { func (m Model) Init() tea.Cmd { return nil } func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { + if completed, ok := message.(refreshDone); ok { + m.refreshing = false + m.snapshot = completed.result.Snapshot + if completed.result.Err != nil { + m.status = "\x1b[31mError: " + completed.result.Err.Error() + "\x1b[0m" + } else { + m.status = completed.result.Notice + } + if m.selected >= len(m.availableTargets()) && m.selected > 0 { + m.selected-- + } + return m, nil + } if progress, ok := message.(profileOperationProgress); ok { m.snapshot = progress.result.Snapshot m.progressTarget = "" @@ -166,15 +184,11 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { m.page = helpPage m.status = "" case "r": - if !m.applying { - result := m.application.Dispatch(app.Intent{Kind: app.RefreshTargets}) - m.snapshot = result.Snapshot - m.status = result.Notice - if result.Err != nil { - m.status = "Error: " + result.Err.Error() - } - if m.selected >= len(m.availableTargets()) && m.selected > 0 { - m.selected-- + if !m.applying && !m.refreshing { + m.refreshing = true + m.status = "Refreshing Target availability and state…" + return m, func() tea.Msg { + return refreshDone{result: m.application.Dispatch(app.Intent{Kind: app.RefreshTargets})} } } case "up", "k": diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index c0c7ba0..78c446e 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -107,12 +107,68 @@ func TestHelpListsEveryTargetAndMainListsOnlyAvailableTargets(t *testing.T) { } } -func TestRefreshKeyDispatchesDetectionRefresh(t *testing.T) { - module := &targetModule{snapshot: app.Snapshot{}, result: app.Result{Snapshot: app.Snapshot{Targets: []app.TargetSnapshot{{ID: "apt", Description: target.Description{Name: "APT"}, Available: true, State: target.External}}}}} +func TestRefreshKeyRunsAsynchronouslyAndRendersRefreshingThenCompleted(t *testing.T) { + module := &targetModule{snapshot: app.Snapshot{}, result: app.Result{Snapshot: app.Snapshot{Targets: []app.TargetSnapshot{{ID: "apt", Description: target.Description{Name: "APT"}, Available: true, State: target.External}}}, Notice: "Refreshed Target availability and state"}} model := NewModel(module) - model = updateKey(model, "r") - if len(module.intents) != 1 || module.intents[0].Kind != app.RefreshTargets || !strings.Contains(model.View(), "External") { - t.Fatalf("refresh = intents %#v\n%s", module.intents, model.View()) + updated, command := model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'r'}}) + model = updated.(Model) + if command == nil || len(module.intents) != 0 || !strings.Contains(model.View(), "Refreshing Target availability and state") { + t.Fatalf("refresh did not start asynchronously: intents %#v\n%s", module.intents, model.View()) + } + message := command() + if len(module.intents) != 1 || module.intents[0].Kind != app.RefreshTargets { + t.Fatalf("refresh command intents = %#v", module.intents) + } + updated, command = model.Update(message) + model = updated.(Model) + if command != nil || !strings.Contains(model.View(), "External") || !strings.Contains(model.View(), "Refreshed Target availability and state") { + t.Fatalf("refresh completion:\n%s", model.View()) + } +} + +func TestTargetPanelNavigationKeysAndQuitCombinations(t *testing.T) { + module := &targetModule{snapshot: app.Snapshot{Targets: []app.TargetSnapshot{ + {ID: "first", Description: target.Description{Name: "First", Summary: "first summary"}, Available: true}, + {ID: "second", Description: target.Description{Name: "Second", Summary: "second summary"}, Available: true}, + }}} + for _, down := range []tea.KeyMsg{{Type: tea.KeyDown}, {Type: tea.KeyRunes, Runes: []rune{'j'}}} { + model := NewModel(module) + updated, _ := model.Update(down) + model = updated.(Model) + if !strings.Contains(model.View(), "> [ ] Second") { + t.Fatalf("down key %q did not select second Target:\n%s", down.String(), model.View()) + } + for _, up := range []tea.KeyMsg{{Type: tea.KeyUp}, {Type: tea.KeyRunes, Runes: []rune{'k'}}} { + updated, _ = model.Update(up) + if !strings.Contains(updated.(Model).View(), "> [ ] First") { + t.Fatalf("up key %q did not select first Target:\n%s", up.String(), updated.(Model).View()) + } + } + } + for _, quit := range []tea.KeyMsg{{Type: tea.KeyRunes, Runes: []rune{'q'}}, {Type: tea.KeyEsc}} { + _, command := NewModel(module).Update(quit) + if command == nil { + t.Fatalf("quit key %q returned no command", quit.String()) + } + if _, ok := command().(tea.QuitMsg); !ok { + t.Fatalf("quit key %q command did not return tea.QuitMsg", quit.String()) + } + } +} + +func TestQAndEscapeReturnFromNestedPages(t *testing.T) { + module := &targetModule{} + for _, key := range []tea.KeyMsg{{Type: tea.KeyRunes, Runes: []rune{'q'}}, {Type: tea.KeyEsc}} { + model := updateKey(NewModel(module), "?") + updated, command := model.Update(key) + if command != nil || !strings.Contains(updated.(Model).View(), "Target panel") { + t.Fatalf("help key %q did not return to Target panel", key.String()) + } + model = updateKey(NewModel(module), "p") + updated, command = model.Update(key) + if command != nil || !strings.Contains(updated.(Model).View(), "Target panel") { + t.Fatalf("Profile key %q did not return to Target panel", key.String()) + } } }