From 7c0b4481d7dca3e9f62f80031cb5d49d08ace230 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 22:23:31 +0800 Subject: [PATCH] feat: unify interactive operation coordination --- .../04-operation-coordination-and-status.md | 28 +- internal/app/app.go | 147 ++--- internal/app/coordination_test.go | 46 +- internal/app/operation.go | 314 ++++++----- internal/app/operation_stream_test.go | 133 ++++- internal/app/progressive_detection_test.go | 77 +-- internal/app/target_test.go | 2 +- internal/tui/dashboard_view.go | 20 +- internal/tui/foundation_test.go | 2 +- internal/tui/model.go | 528 ++++-------------- internal/tui/model_test.go | 9 +- internal/tui/operation_coordination_test.go | 102 +++- internal/tui/operation_coordinator.go | 198 +++++++ internal/tui/pages.go | 30 +- internal/tui/presentation.go | 42 +- internal/tui/progressive_detection_test.go | 70 ++- internal/tui/target_test.go | 250 ++------- 17 files changed, 985 insertions(+), 1013 deletions(-) create mode 100644 internal/tui/operation_coordinator.go diff --git a/.scratch/tui-redesign/issues/04-operation-coordination-and-status.md b/.scratch/tui-redesign/issues/04-operation-coordination-and-status.md index 55092d3..d7c4012 100644 --- a/.scratch/tui-redesign/issues/04-operation-coordination-and-status.md +++ b/.scratch/tui-redesign/issues/04-operation-coordination-and-status.md @@ -4,18 +4,18 @@ **Blocked by:** 03 — Stream progressive startup and refresh. -**Status:** ready-for-agent +**Status:** done -- [ ] Detection, Target mutation, and Active Profile transition use one presentation-independent progressive operation contract where their semantics overlap. -- [ ] System mutations remain serialized and a conflicting action exposes an actionable disabled reason instead of silently ignoring input. -- [ ] During a mutation, users can still select Targets, scroll details, and open existing read-only Help. -- [ ] The affected Target row displays progress while the status area describes the current step. -- [ ] Successful status is replaceable by later activity. -- [ ] Current errors remain visible until acknowledged, superseded by explicit resolution, or replaced by a directly related result. -- [ ] Current error details can be expanded without introducing an activity-history page. -- [ ] Partial Active Profile transitions retain successful Target updates, preserve the no-cross-Target-rollback rule, and focus the first failed Target. -- [ ] Confirmation state, progress, and final result are structured; the TUI does not parse user-facing strings to infer operation state. -- [ ] `q` on the dashboard and Ctrl+C on any page request exit; during a mutation the UI visibly reports deferred exit and exits only after safe completion. -- [ ] No new work can begin after deferred exit is requested. -- [ ] The concrete Application remains usable by existing synchronous callers while obsolete interactive fallback paths are removed after migration. -- [ ] Application and complete-Model tests cover browsing while busy, blocked mutations, persistent errors, partial failure, confirmation, and deferred exit. +- [x] Detection, Target mutation, and Active Profile transition use one presentation-independent progressive operation contract where their semantics overlap. +- [x] System mutations remain serialized and a conflicting action exposes an actionable disabled reason instead of silently ignoring input. +- [x] During a mutation, users can still select Targets, scroll details, and open existing read-only Help. +- [x] The affected Target row displays progress while the status area describes the current step. +- [x] Successful status is replaceable by later activity. +- [x] Current errors remain visible until acknowledged, superseded by explicit resolution, or replaced by a directly related result. +- [x] Current error details can be expanded without introducing an activity-history page. +- [x] Partial Active Profile transitions retain successful Target updates, preserve the no-cross-Target-rollback rule, and focus the first failed Target. +- [x] Confirmation state, progress, and final result are structured; the TUI does not parse user-facing strings to infer operation state. +- [x] `q` on the dashboard and Ctrl+C on any page request exit; during a mutation the UI visibly reports deferred exit and exits only after safe completion. +- [x] No new work can begin after deferred exit is requested. +- [x] The concrete Application remains usable by existing synchronous callers while obsolete interactive fallback paths are removed after migration. +- [x] Application and complete-Model tests cover browsing while busy, blocked mutations, persistent errors, partial failure, confirmation, and deferred exit. diff --git a/internal/app/app.go b/internal/app/app.go index 26483ac..2eb0456 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -53,15 +53,6 @@ type TargetSnapshot struct { Evidence string } -// TargetDetectionUpdate carries the complete authoritative Application state -// after one Target changed. TargetID identifies that change without duplicating -// availability, inspection, or failure fields already present in Snapshot. -type TargetDetectionUpdate struct { - Snapshot Snapshot - TargetID string - Complete bool -} - type Snapshot struct { Profiles []profile.Profile ActiveProfile string @@ -70,9 +61,6 @@ type Snapshot struct { type TargetProgress struct { TargetID string - // Applying is retained for synchronous DispatchProgress callers. New - // interactive callers use the structured Phase field. - Applying bool Phase TargetProgressPhase State target.State Evidence string @@ -80,15 +68,12 @@ type TargetProgress struct { } type Result struct { - Snapshot Snapshot - Notice string - // Confirmation remains for existing synchronous callers. - Confirmation string + Snapshot Snapshot + Notice string ConfirmationRequest *Confirmation Status OperationStatus Progress []TargetProgress FirstFailedTargetID string - Complete bool Err error } @@ -97,20 +82,6 @@ type Module interface { Dispatch(Intent) Result } -// ProgressiveModule is implemented by Applications that can expose each -// coordinated Target update while an Active Profile transition is running. -// The final Result has Complete set and closes the stream. -type ProgressiveModule interface { - DispatchProgress(Intent) <-chan Result -} - -// ProgressiveDetectionModule exposes a useful all-Checking snapshot -// synchronously, followed by one structured Target result at a time. Startup -// and explicit refresh use this same presentation-independent contract. -type ProgressiveDetectionModule interface { - BeginTargetDetection() (Snapshot, <-chan TargetDetectionUpdate) -} - type Application struct { store profile.Store state profile.State @@ -196,7 +167,13 @@ type targetDetectionOutcome struct { detection target.Detection } -func (a *Application) BeginTargetDetection() (Snapshot, <-chan TargetDetectionUpdate) { +type targetDetectionUpdate struct { + snapshot Snapshot + targetID string + complete bool +} + +func (a *Application) beginTargetDetection() (Snapshot, <-chan targetDetectionUpdate) { a.stateMu.Lock() a.detectionGeneration++ generation := a.detectionGeneration @@ -208,7 +185,7 @@ func (a *Application) BeginTargetDetection() (Snapshot, <-chan TargetDetectionUp checking := a.snapshotLocked() a.stateMu.Unlock() - updates := make(chan TargetDetectionUpdate, len(a.targets)+1) + updates := make(chan targetDetectionUpdate, len(a.targets)+1) outcomes := make(chan targetDetectionOutcome, len(a.targets)) for i, adapter := range a.targets { adapter, id := adapter, a.targetIDs[i] @@ -233,7 +210,7 @@ func (a *Application) BeginTargetDetection() (Snapshot, <-chan TargetDetectionUp a.inspections[outcome.targetID] = outcome.detection.Inspection a.detectionPhase[outcome.targetID] = phase a.detectionErr[outcome.targetID] = outcome.detection.Err - update := TargetDetectionUpdate{Snapshot: a.snapshotLocked(), TargetID: outcome.targetID} + update := targetDetectionUpdate{snapshot: a.snapshotLocked(), targetID: outcome.targetID} // The buffered send cannot wait on a consumer and happens before a // newer Begin call can make this generation stale. updates <- update @@ -241,7 +218,7 @@ func (a *Application) BeginTargetDetection() (Snapshot, <-chan TargetDetectionUp } a.stateMu.Lock() if generation == a.detectionGeneration { - updates <- TargetDetectionUpdate{Snapshot: a.snapshotLocked(), Complete: true} + updates <- targetDetectionUpdate{snapshot: a.snapshotLocked(), complete: true} } a.stateMu.Unlock() }() @@ -262,17 +239,6 @@ func (a *Application) Dispatch(intent Intent) Result { return a.dispatch(intent, nil) } -func (a *Application) DispatchProgress(intent Intent) <-chan Result { - results := make(chan Result, 2*len(a.targets)+1) - go func() { - defer close(results) - final := a.dispatch(intent, func(update Result) { results <- update }) - final.Complete = true - results <- final - }() - return results -} - func (a *Application) dispatch(intent Intent, publish func(Result)) Result { // Serialize local mutation workflows even when tests intentionally omit the // cross-process MutationLock. @@ -302,8 +268,7 @@ func (a *Application) dispatch(intent Intent, publish func(Result)) Result { return a.setTargetDesired(intent, publish) } if intent.Kind == RefreshTargets { - a.refreshTargets() - return Result{Snapshot: a.Snapshot(), Notice: "Refreshed Target availability and state"} + return a.refreshTargets(publish) } if intent.Kind == ActivateProfile { return a.activateProfile(intent, publish) @@ -371,8 +336,9 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul Title: "Confirm Active Profile change", Summary: "Changing the Active Profile has high-impact Target effects.", Details: warnings, ConfirmLabel: "Change Profile", CancelLabel: "Cancel", HighImpact: true, } - return Result{Snapshot: a.Snapshot(), Confirmation: message, ConfirmationRequest: confirmation, - Status: OperationStatus{Kind: StatusConfirmation, Summary: confirmation.Title, Detail: confirmation.Summary}} + _ = message + return Result{Snapshot: a.Snapshot(), ConfirmationRequest: confirmation, + Status: OperationStatus{Summary: confirmation.Title}} } next, err := a.store.Mutate(func(current profile.State) (profile.State, error) { @@ -410,8 +376,8 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul name := a.targetDescriptions[targetIndex].Name updated[id] = true if publish != nil { - publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{{TargetID: id, Applying: true, Phase: TargetProgressRunning, State: a.inspection(id).State}}, Notice: "Updating " + name + " Target", - Status: OperationStatus{Kind: StatusProgress, Summary: "Updating " + name + " Target", TargetID: id}}) + publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{{TargetID: id, Phase: TargetProgressRunning, State: a.inspection(id).State}}, Notice: "Updating " + name + " Target", + Status: OperationStatus{Summary: "Updating " + name + " Target", TargetID: id}}) } operation := adapter.SetDesired(context.Background(), true, active) inspection := operation.Inspection @@ -430,9 +396,9 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul item := TargetProgress{TargetID: id, Phase: progressPhase, State: inspection.State, Evidence: inspection.Evidence, Err: safeErr} progress = append(progress, item) if publish != nil { - status := OperationStatus{Kind: StatusSuccess, Summary: name + " Target update completed", TargetID: id} + status := OperationStatus{Summary: name + " Target update completed", TargetID: id} if safeErr != nil { - status = OperationStatus{Kind: StatusError, Summary: name + " Target update failed", Detail: safeErr.Error(), TargetID: id} + status = OperationStatus{Summary: name + " Target update failed", TargetID: id} } publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{item}, Notice: name + " Target update completed", Status: status, Err: safeErr}) } @@ -458,11 +424,11 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul } a.mergeInspections(remaining) result := Result{Snapshot: a.Snapshot(), Progress: progress, Notice: fmt.Sprintf("Activated Profile %q", intent.Name), - Status: OperationStatus{Kind: StatusSuccess, Summary: fmt.Sprintf("Activated Profile %q", intent.Name)}} + Status: OperationStatus{Summary: fmt.Sprintf("Activated Profile %q", intent.Name)}} if len(failures) > 0 { result.Err = errors.Join(failures...) result.Notice = fmt.Sprintf("Activated Profile %q with %d Target update failure(s); successful updates were kept and no global rollback was attempted", intent.Name, len(failures)) - result.Status = OperationStatus{Kind: StatusError, Summary: fmt.Sprintf("Activated Profile %q with %d Target update failure(s)", intent.Name, len(failures)), Detail: result.Err.Error()} + result.Status = OperationStatus{Summary: fmt.Sprintf("Activated Profile %q with %d Target update failure(s)", intent.Name, len(failures))} for _, item := range progress { if item.Err != nil { result.FirstFailedTargetID = item.TargetID @@ -548,10 +514,61 @@ func (a *Application) mergeInspections(inspections map[string]target.Inspection) } } -func (a *Application) refreshTargets() { - _, updates := a.BeginTargetDetection() - for range updates { +func targetByID(snapshot Snapshot, id string) (TargetSnapshot, string) { + for _, item := range snapshot.Targets { + if item.ID == id { + name := item.Description.Name + if name == "" { + name = id + } + return item, name + } } + return TargetSnapshot{ID: id}, id +} + +func (a *Application) refreshTargets(publish func(Result)) Result { + checking, updates := a.beginTargetDetection() + if publish != nil { + progress := make([]TargetProgress, 0, len(checking.Targets)) + for _, item := range checking.Targets { + progress = append(progress, TargetProgress{TargetID: item.ID, Phase: TargetProgressRunning, State: item.State}) + } + publish(Result{Snapshot: checking, Status: OperationStatus{Summary: "Checking Target availability and state"}, Progress: progress}) + } + var failures []error + firstFailed := "" + final := checking + for update := range updates { + final = update.snapshot + if update.complete { + continue + } + item, name := targetByID(update.snapshot, update.targetID) + progress := TargetProgress{TargetID: update.targetID, Phase: TargetProgressSucceeded, State: item.State, Evidence: item.Evidence} + status := OperationStatus{Summary: "Checked " + name + " Target", TargetID: update.targetID} + if item.DetectionPhase == TargetDetectionFailed { + progress.Phase = TargetProgressFailed + progress.Err = item.DetectionErr + status.Summary = "Could not check " + name + " Target" + if item.DetectionErr != nil { + failures = append(failures, fmt.Errorf("%s Target: %w", name, item.DetectionErr)) + } + if firstFailed == "" { + firstFailed = update.targetID + } + } + if publish != nil { + publish(Result{Snapshot: update.snapshot, Status: status, Progress: []TargetProgress{progress}, Err: progress.Err}) + } + } + result := Result{Snapshot: final, Notice: "Target availability and state checks completed", Status: OperationStatus{Summary: "Target availability and state checks completed"}} + if len(failures) > 0 { + result.Err = errors.Join(failures...) + result.FirstFailedTargetID = firstFailed + result.Status = OperationStatus{Summary: "Some Target checks failed", TargetID: firstFailed} + } + return result } func (a *Application) refreshInspections() { @@ -582,12 +599,12 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu if warning := selected.Confirmation(intent.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(), Confirmation: warning, ConfirmationRequest: confirmation, - Status: OperationStatus{Kind: StatusConfirmation, Summary: confirmation.Title, Detail: confirmation.Summary, TargetID: intent.TargetID}} + 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, Applying: true, Phase: TargetProgressRunning, State: a.inspection(intent.TargetID).State}}, - Status: OperationStatus{Kind: StatusProgress, Summary: "Updating " + a.targetDescriptions[selectedIndex].Name + " Target", TargetID: intent.TargetID}}) + 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) inspection := result.Inspection @@ -608,15 +625,13 @@ func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Resu } notice := "" progressPhase := TargetProgressSucceeded - status := OperationStatus{Kind: StatusSuccess, TargetID: intent.TargetID} + status := OperationStatus{TargetID: intent.TargetID} if result.Err == nil { notice = fmt.Sprintf("%s Target is now %s", a.targetDescriptions[selectedIndex].Name, inspection.State) status.Summary = notice } else { progressPhase = TargetProgressFailed - status.Kind = StatusError status.Summary = a.targetDescriptions[selectedIndex].Name + " Target update failed" - status.Detail = result.Err.Error() } progress := TargetProgress{TargetID: intent.TargetID, Phase: progressPhase, State: inspection.State, Evidence: inspection.Evidence, Err: result.Err} 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} diff --git a/internal/app/coordination_test.go b/internal/app/coordination_test.go index 0b6ad10..358929e 100644 --- a/internal/app/coordination_test.go +++ b/internal/app/coordination_test.go @@ -126,27 +126,31 @@ func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing. } 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 { + var final app.OperationUpdate + var failedCompletion app.OperationUpdate + stream := application.StartOperation(app.Intent{Kind: app.ActivateProfile, Name: "new"}) + for update := range stream.Updates { + final = update + progress := update.Progress() + if len(progress) == 1 && progress[0].TargetID == "second" && progress[0].Phase == app.TargetProgressFailed { failedCompletion = update } } - if result.Snapshot.ActiveProfile != "new" { - t.Fatalf("Active Profile = %q", result.Snapshot.ActiveProfile) + if final.Snapshot.ActiveProfile != "new" { + t.Fatalf("Active Profile = %q", final.Snapshot.ActiveProfile) } - if result.Err == nil || strings.Contains(result.Err.Error(), "secret") || !strings.Contains(result.Err.Error(), "second") { - t.Fatalf("redacted partial failure = %v", result.Err) + if final.Err() == nil || strings.Contains(final.Err().Error(), "secret") || !strings.Contains(final.Err().Error(), "second") { + t.Fatalf("redacted partial failure = %v", final.Err()) } - if got := result.Snapshot.Targets; got[0].State != target.Enabled || got[1].State != target.Drifted { + if got := final.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 || result.Progress[1].Evidence != "fresh actual configuration" { - t.Fatalf("per-Target progress = %#v", result.Progress) + progress := final.Progress() + if len(progress) != 2 || progress[0].TargetID != "first" || progress[0].State != target.Enabled || progress[1].TargetID != "second" || progress[1].State != target.Drifted || progress[1].Evidence != "fresh actual configuration" { + t.Fatalf("per-Target progress = %#v", progress) } - if len(failedCompletion.Progress) != 1 || failedCompletion.Progress[0].State != target.Drifted || failedCompletion.Progress[0].Evidence != "fresh actual configuration" || failedCompletion.Err == nil { + failedProgress := failedCompletion.Progress() + if len(failedProgress) != 1 || failedProgress[0].State != target.Drifted || failedProgress[0].Evidence != "fresh actual configuration" || failedProgress[0].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" { @@ -155,8 +159,8 @@ func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing. 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 !strings.Contains(final.Err().Error(), "fresh actual configuration") || !strings.Contains(final.Err().Error(), "Drifted") { + t.Fatalf("failure summary omitted fresh inspection: %v", final.Err()) } if len(first.applied) != 2 || first.applied[1] != "new" { t.Fatalf("successful Target was not updated: %#v", first.applied) @@ -231,7 +235,7 @@ func TestActiveProfileProgressReusesAvailabilityCache(t *testing.T) { callsBeforeSwitch := adapter.availabilityCalls var updates int - for range application.DispatchProgress(app.Intent{Kind: app.ActivateProfile, Name: "new"}) { + for range application.StartOperation(app.Intent{Kind: app.ActivateProfile, Name: "new"}).Updates { updates++ } if updates < 3 { @@ -264,17 +268,19 @@ func TestActiveProfileSwitchStreamsApplyingAndCompletionForEachTarget(t *testing t.Fatal(result.Err) } - updates := application.DispatchProgress(app.Intent{Kind: app.ActivateProfile, Name: "new"}) + updates := application.StartOperation(app.Intent{Kind: app.ActivateProfile, Name: "new"}).Updates applying := <-updates completed := <-updates final := <-updates - if len(applying.Progress) != 1 || !applying.Progress[0].Applying || applying.Progress[0].TargetID != "shell" { + applyingProgress := applying.Progress() + if len(applyingProgress) != 1 || applyingProgress[0].Phase != app.TargetProgressRunning || applyingProgress[0].TargetID != "shell" { t.Fatalf("applying update = %#v", applying) } - if len(completed.Progress) != 1 || completed.Progress[0].Applying || completed.Progress[0].State != target.Enabled { + completedProgress := completed.Progress() + if len(completedProgress) != 1 || completedProgress[0].Phase != app.TargetProgressSucceeded || completedProgress[0].State != target.Enabled { t.Fatalf("completion update = %#v", completed) } - if !final.Complete || final.Snapshot.ActiveProfile != "new" { + if !final.IsSucceeded() || final.Snapshot.ActiveProfile != "new" { t.Fatalf("final update = %#v", final) } } diff --git a/internal/app/operation.go b/internal/app/operation.go index 969b91d..994c1a7 100644 --- a/internal/app/operation.go +++ b/internal/app/operation.go @@ -1,21 +1,5 @@ package app -import ( - "errors" - "fmt" -) - -// OperationPhase describes coordination state independently of any terminal UI. -type OperationPhase uint8 - -const ( - OperationRunning OperationPhase = iota + 1 - OperationAwaitingConfirmation - OperationSucceeded - OperationFailed - OperationBlocked -) - // StatusKind gives presentation layers semantics without requiring them to // inspect user-facing status text. type StatusKind uint8 @@ -25,15 +9,12 @@ const ( StatusSuccess StatusError StatusBlocked - StatusConfirmation ) -// OperationStatus is the current concise status plus optional expandable -// detail. TargetID relates a result to the Target it can explicitly resolve. +// OperationStatus is concise status text related to an optional Target. type OperationStatus struct { Kind StatusKind Summary string - Detail string TargetID string } @@ -57,21 +38,161 @@ const ( TargetProgressFailed ) -// OperationUpdate is the common progressive contract used by interactive -// detection, Target mutation, and Active Profile transition operations. -type OperationUpdate struct { - Snapshot Snapshot - Phase OperationPhase - Status OperationStatus - Progress []TargetProgress - Confirmation *Confirmation - FirstFailedTargetID string - Err error +// OperationState is a closed tagged set. Its concrete value makes an update +// exactly one of running, confirmation, succeeded, failed, or closed. +type OperationState interface { + operationState() } +type operationRunning struct { + status OperationStatus + progress []TargetProgress +} + +func (operationRunning) operationState() {} + +type operationConfirmation struct{ request Confirmation } + +func (operationConfirmation) operationState() {} + +type operationSucceeded struct { + status OperationStatus + progress []TargetProgress +} + +func (operationSucceeded) operationState() {} + +type operationFailed struct { + status OperationStatus + progress []TargetProgress + firstFailedTargetID string + err error +} + +func (operationFailed) operationState() {} + +type operationClosed struct{ status OperationStatus } + +func (operationClosed) operationState() {} + +// OperationUpdate is the common progressive contract used by interactive +// detection, Target mutation, and Active Profile transition operations. Build +// values with the constructors below so contradictory states are unrepresentable. +type OperationUpdate struct { + Snapshot Snapshot + State OperationState +} + +func RunningOperationUpdate(snapshot Snapshot, status OperationStatus, progress []TargetProgress) OperationUpdate { + status.Kind = StatusProgress + return OperationUpdate{Snapshot: snapshot, State: operationRunning{status: status, progress: cloneProgress(progress)}} +} + +func ConfirmationOperationUpdate(snapshot Snapshot, request Confirmation) OperationUpdate { + return OperationUpdate{Snapshot: snapshot, State: operationConfirmation{request: request}} +} + +func SucceededOperationUpdate(snapshot Snapshot, status OperationStatus, progress []TargetProgress) OperationUpdate { + status.Kind = StatusSuccess + return OperationUpdate{Snapshot: snapshot, State: operationSucceeded{status: status, progress: cloneProgress(progress)}} +} + +func FailedOperationUpdate(snapshot Snapshot, status OperationStatus, progress []TargetProgress, firstFailedTargetID string, err error) OperationUpdate { + status.Kind = StatusError + return OperationUpdate{Snapshot: snapshot, State: operationFailed{ + status: status, progress: cloneProgress(progress), firstFailedTargetID: firstFailedTargetID, err: err, + }} +} + +// ClosedOperationUpdate reports a request that cannot start, such as an +// overlapping interactive operation. It is terminal and has no progress or +// error payload. +func ClosedOperationUpdate(snapshot Snapshot, status OperationStatus) OperationUpdate { + status.Kind = StatusBlocked + return OperationUpdate{Snapshot: snapshot, State: operationClosed{status: status}} +} + +func cloneProgress(progress []TargetProgress) []TargetProgress { + return append([]TargetProgress(nil), progress...) +} + +func (u OperationUpdate) Status() OperationStatus { + switch state := u.State.(type) { + case operationRunning: + return state.status + case operationSucceeded: + return state.status + case operationFailed: + return state.status + case operationClosed: + return state.status + default: + return OperationStatus{} + } +} + +func (u OperationUpdate) Progress() []TargetProgress { + switch state := u.State.(type) { + case operationRunning: + return cloneProgress(state.progress) + case operationSucceeded: + return cloneProgress(state.progress) + case operationFailed: + return cloneProgress(state.progress) + default: + return nil + } +} + +func (u OperationUpdate) Confirmation() *Confirmation { + state, ok := u.State.(operationConfirmation) + if !ok { + return nil + } + request := state.request + request.Details = append([]string(nil), request.Details...) + return &request +} + +func (u OperationUpdate) Err() error { + if state, ok := u.State.(operationFailed); ok { + return state.err + } + return nil +} + +func (u OperationUpdate) FirstFailedTargetID() string { + if state, ok := u.State.(operationFailed); ok { + return state.firstFailedTargetID + } + return "" +} + +func (u OperationUpdate) IsRunning() bool { + _, ok := u.State.(operationRunning) + return ok +} + +func (u OperationUpdate) IsConfirmation() bool { + _, ok := u.State.(operationConfirmation) + return ok +} + +func (u OperationUpdate) IsSucceeded() bool { + _, ok := u.State.(operationSucceeded) + return ok +} + +func (u OperationUpdate) IsFailed() bool { + _, ok := u.State.(operationFailed) + return ok +} + +func (u OperationUpdate) IsTerminal() bool { return !u.IsRunning() } + // OperationStream exposes the useful initial frame synchronously and all // subsequent structured changes through Updates. Updates closes after a -// terminal phase. +// terminal update. type OperationStream struct { Snapshot Snapshot Updates <-chan OperationUpdate @@ -84,84 +205,32 @@ type InteractiveModule interface { StartOperation(Intent) OperationStream } -// StartOperation rejects overlap instead of queuing hidden interactive work. -// Synchronous Dispatch callers retain the established serialized behavior. +// StartOperation returns before dispatch serialization is acquired. This keeps +// Bubble Tea Update non-blocking even when a synchronous caller currently owns +// the serialized Application entry. func (a *Application) StartOperation(intent Intent) OperationStream { + initial := a.Snapshot() a.interactiveMu.Lock() if a.interactiveOperation { a.interactiveMu.Unlock() updates := make(chan OperationUpdate, 1) - updates <- OperationUpdate{ - Snapshot: a.Snapshot(), - Phase: OperationBlocked, - Status: OperationStatus{Kind: StatusBlocked, - Summary: "Another system operation is already in progress", - Detail: "Wait for the current operation to finish before starting another modification."}, - } + updates <- ClosedOperationUpdate(initial, OperationStatus{ + Summary: "Another system operation is already in progress", + }) close(updates) - return OperationStream{Snapshot: a.Snapshot(), Updates: updates} + return OperationStream{Snapshot: initial, Updates: updates} } a.interactiveOperation = true a.interactiveMu.Unlock() - if intent.Kind == RefreshTargets { - return a.startDetectionOperation() - } - - initial := a.Snapshot() updates := make(chan OperationUpdate, 2*len(a.targets)+2) go func() { defer close(updates) defer a.finishInteractiveOperation() final := a.dispatch(intent, func(result Result) { - updates <- operationUpdate(result, false) + updates <- runningOperationUpdate(result) }) - updates <- operationUpdate(final, true) - }() - return OperationStream{Snapshot: initial, Updates: updates} -} - -func (a *Application) startDetectionOperation() OperationStream { - initial, detection := a.BeginTargetDetection() - updates := make(chan OperationUpdate, len(a.targets)+1) - go func() { - defer close(updates) - defer a.finishInteractiveOperation() - var failures []error - firstFailed := "" - for item := range detection { - if item.Complete { - update := OperationUpdate{ - Snapshot: item.Snapshot, - Phase: OperationSucceeded, - Status: OperationStatus{Kind: StatusSuccess, Summary: "Target availability and state checks completed"}, - } - if len(failures) > 0 { - update.Phase = OperationFailed - update.Err = errors.Join(failures...) - update.FirstFailedTargetID = firstFailed - update.Status = OperationStatus{Kind: StatusError, Summary: "Some Target checks failed", Detail: update.Err.Error(), TargetID: firstFailed} - } - updates <- update - continue - } - targetSnapshot, name := targetByID(item.Snapshot, item.TargetID) - progress := TargetProgress{TargetID: item.TargetID, State: targetSnapshot.State, Evidence: targetSnapshot.Evidence, Phase: TargetProgressSucceeded} - status := OperationStatus{Kind: StatusProgress, Summary: "Checked " + name + " Target", TargetID: item.TargetID} - if targetSnapshot.DetectionPhase == TargetDetectionFailed { - progress.Phase = TargetProgressFailed - progress.Err = targetSnapshot.DetectionErr - status = OperationStatus{Kind: StatusError, Summary: "Could not check " + name + " Target", TargetID: item.TargetID} - if targetSnapshot.DetectionErr != nil { - status.Detail = targetSnapshot.DetectionErr.Error() - failures = append(failures, fmt.Errorf("%s Target: %w", name, targetSnapshot.DetectionErr)) - } - if firstFailed == "" { - firstFailed = item.TargetID - } - } - updates <- OperationUpdate{Snapshot: item.Snapshot, Phase: OperationRunning, Status: status, Progress: []TargetProgress{progress}, Err: progress.Err} - } + updates <- terminalOperationUpdate(final) }() return OperationStream{Snapshot: initial, Updates: updates} } @@ -172,57 +241,26 @@ func (a *Application) finishInteractiveOperation() { a.interactiveMu.Unlock() } -func targetByID(snapshot Snapshot, id string) (TargetSnapshot, string) { - for _, item := range snapshot.Targets { - if item.ID == id { - name := item.Description.Name - if name == "" { - name = id - } - return item, name - } +func runningOperationUpdate(result Result) OperationUpdate { + if result.ConfirmationRequest != nil { + return ConfirmationOperationUpdate(result.Snapshot, *result.ConfirmationRequest) } - return TargetSnapshot{ID: id}, id + return RunningOperationUpdate(result.Snapshot, result.Status, result.Progress) } -func operationUpdate(result Result, terminal bool) OperationUpdate { - update := OperationUpdate{ - Snapshot: result.Snapshot, - Phase: OperationRunning, - Status: result.Status, - Progress: result.Progress, - Confirmation: result.ConfirmationRequest, - FirstFailedTargetID: result.FirstFailedTargetID, - Err: result.Err, - } +func terminalOperationUpdate(result Result) OperationUpdate { if result.ConfirmationRequest != nil { - update.Phase = OperationAwaitingConfirmation - if update.Status.Kind == 0 { - update.Status = OperationStatus{Kind: StatusConfirmation, Summary: result.ConfirmationRequest.Title, Detail: result.ConfirmationRequest.Summary} - } - return update - } - if update.Status.Kind == 0 { - switch { - case result.Err != nil: - update.Status = OperationStatus{Kind: StatusError, Summary: "Operation failed", Detail: result.Err.Error()} - case result.Notice != "": - update.Status = OperationStatus{Kind: StatusSuccess, Summary: result.Notice} - } - } - if !terminal { - return update + return ConfirmationOperationUpdate(result.Snapshot, *result.ConfirmationRequest) } + status := result.Status if result.Err != nil { - update.Phase = OperationFailed - if update.Status.Kind != StatusError { - update.Status = OperationStatus{Kind: StatusError, Summary: "Operation failed", Detail: result.Err.Error(), TargetID: update.Status.TargetID} - } - } else { - update.Phase = OperationSucceeded - if update.Status.Kind == StatusProgress || update.Status.Kind == 0 { - update.Status.Kind = StatusSuccess + if status.Summary == "" { + status.Summary = "Operation failed" } + return FailedOperationUpdate(result.Snapshot, status, result.Progress, result.FirstFailedTargetID, result.Err) } - return update + if status.Summary == "" { + status.Summary = result.Notice + } + return SucceededOperationUpdate(result.Snapshot, status, result.Progress) } diff --git a/internal/app/operation_stream_test.go b/internal/app/operation_stream_test.go index 0d31e3c..d311c48 100644 --- a/internal/app/operation_stream_test.go +++ b/internal/app/operation_stream_test.go @@ -5,7 +5,9 @@ import ( "errors" "path/filepath" "strings" + "sync" "testing" + "time" "proxyctl/internal/app" "proxyctl/internal/profile" @@ -13,12 +15,17 @@ import ( ) type coordinatedOperationTarget struct { - id string - state target.State - started chan struct{} - release chan struct{} - confirmation string - failure error + id string + state target.State + started chan struct{} + release chan struct{} + detectionStarted chan struct{} + detectionRelease chan struct{} + confirmation string + failure error + mu sync.Mutex + inDetection bool + overlapObserved chan bool } func (t *coordinatedOperationTarget) ID() string { return t.id } @@ -32,7 +39,28 @@ func (t *coordinatedOperationTarget) Inspect(context.Context, *profile.Profile) return target.Inspection{State: t.state, Evidence: "fresh state"} } func (t *coordinatedOperationTarget) Confirmation(bool) string { return t.confirmation } +func (t *coordinatedOperationTarget) Detect(ctx context.Context, active *profile.Profile) target.Detection { + t.mu.Lock() + t.inDetection = true + t.mu.Unlock() + if t.detectionStarted != nil { + close(t.detectionStarted) + } + if t.detectionRelease != nil { + <-t.detectionRelease + } + t.mu.Lock() + t.inDetection = false + t.mu.Unlock() + return target.Detection{Availability: t.Availability(ctx), Inspection: t.Inspect(ctx, active)} +} func (t *coordinatedOperationTarget) SetDesired(_ context.Context, enabled bool, _ *profile.Profile) target.OperationResult { + t.mu.Lock() + overlapped := t.inDetection + t.mu.Unlock() + if t.overlapObserved != nil { + t.overlapObserved <- overlapped + } if t.started != nil { close(t.started) } @@ -66,14 +94,15 @@ func TestInteractiveOperationStreamSerializesMutationAndPublishesStructuredProgr operation := application.StartOperation(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}) running := <-operation.Updates - if running.Phase != app.OperationRunning || len(running.Progress) != 1 || running.Progress[0].Phase != app.TargetProgressRunning || running.Progress[0].TargetID != "shell" || running.Status.Kind != app.StatusProgress { + runningProgress := running.Progress() + if !running.IsRunning() || len(runningProgress) != 1 || runningProgress[0].Phase != app.TargetProgressRunning || runningProgress[0].TargetID != "shell" || running.Status().Kind != app.StatusProgress { t.Fatalf("running update = %#v", running) } <-adapter.started blocked := application.StartOperation(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: false}) blockedUpdate := <-blocked.Updates - if blockedUpdate.Phase != app.OperationBlocked || blockedUpdate.Status.Kind != app.StatusBlocked || !strings.Contains(blockedUpdate.Status.Summary, "already in progress") { + if !blockedUpdate.IsTerminal() || blockedUpdate.Status().Kind != app.StatusBlocked || !strings.Contains(blockedUpdate.Status().Summary, "already in progress") { t.Fatalf("blocked update = %#v", blockedUpdate) } if _, open := <-blocked.Updates; open { @@ -82,7 +111,8 @@ func TestInteractiveOperationStreamSerializesMutationAndPublishesStructuredProgr close(adapter.release) final := <-operation.Updates - if final.Phase != app.OperationSucceeded || final.Snapshot.Targets[0].State != target.Enabled || len(final.Progress) != 1 || final.Progress[0].Phase != app.TargetProgressSucceeded { + finalProgress := final.Progress() + if !final.IsSucceeded() || final.Snapshot.Targets[0].State != target.Enabled || len(finalProgress) != 1 || finalProgress[0].Phase != app.TargetProgressSucceeded { t.Fatalf("final update = %#v", final) } if _, open := <-operation.Updates; open { @@ -90,6 +120,86 @@ func TestInteractiveOperationStreamSerializesMutationAndPublishesStructuredProgr } } +func TestDetectionOperationSerializesWithSynchronousMutation(t *testing.T) { + adapter := &coordinatedOperationTarget{ + id: "shell", state: target.Disabled, + detectionStarted: make(chan struct{}), detectionRelease: make(chan struct{}), + started: make(chan struct{}), overlapObserved: make(chan bool, 1), + } + 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.CreateProfile, Profile: profile.Profile{Name: "office", SingleURL: "http://proxy.example:8080"}}); result.Err != nil { + t.Fatal(result.Err) + } + if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "office"}); result.Err != nil { + t.Fatal(result.Err) + } + + detection := application.StartOperation(app.Intent{Kind: app.RefreshTargets}) + <-adapter.detectionStarted + mutationAttempted := make(chan struct{}) + mutationDone := make(chan app.Result, 1) + go func() { + close(mutationAttempted) + mutationDone <- application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true}) + }() + <-mutationAttempted + close(adapter.detectionRelease) + for range detection.Updates { + } + result := <-mutationDone + if result.Err != nil || result.Snapshot.Targets[0].State != target.Enabled { + t.Fatalf("serialized mutation = %#v", result) + } + if overlapped := <-adapter.overlapObserved; overlapped { + t.Fatal("mutation overlapped Target detection") + } +} + +func TestStartOperationReturnsBeforeDispatchSerializationIsAvailable(t *testing.T) { + 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 { + t.Fatal(err) + } + if result := application.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{Name: "office", SingleURL: "http://proxy.example:8080"}}); result.Err != nil { + t.Fatal(result.Err) + } + if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "office"}); result.Err != nil { + t.Fatal(result.Err) + } + + 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 + + returned := make(chan app.OperationStream, 1) + go func() { returned <- application.StartOperation(app.Intent{Kind: app.RefreshTargets}) }() + var stream app.OperationStream + select { + case stream = <-returned: + case <-time.After(time.Second): + t.Fatal("StartOperation blocked Bubble Tea while dispatch serialization was held") + } + + close(adapter.release) + if result := <-mutationDone; result.Err != nil { + t.Fatal(result.Err) + } + <-adapter.detectionStarted + close(adapter.detectionRelease) + for range stream.Updates { + } +} + func TestInteractiveOperationStreamUsesStructuredConfirmation(t *testing.T) { adapter := &coordinatedOperationTarget{id: "docker-daemon", state: target.Disabled, confirmation: "Docker will restart and may disrupt running work."} application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) @@ -105,7 +215,8 @@ func TestInteractiveOperationStreamUsesStructuredConfirmation(t *testing.T) { operation := application.StartOperation(app.Intent{Kind: app.SetTargetDesired, TargetID: adapter.ID(), Enabled: true}) update := <-operation.Updates - if update.Phase != app.OperationAwaitingConfirmation || update.Confirmation == nil || update.Confirmation.Title == "" || update.Confirmation.CancelLabel == "" || update.Status.Kind != app.StatusConfirmation { + confirmation := update.Confirmation() + if !update.IsConfirmation() || confirmation == nil || confirmation.Title == "" || confirmation.CancelLabel == "" { t.Fatalf("confirmation update = %#v", update) } if _, open := <-operation.Updates; open { @@ -140,7 +251,7 @@ func TestInteractiveProfileTransitionReportsFirstFailureWithoutRollback(t *testi for update := range operation.Updates { final = update } - if final.Phase != app.OperationFailed || final.FirstFailedTargetID != "second" || final.Snapshot.ActiveProfile != "new" { + if !final.IsFailed() || final.FirstFailedTargetID() != "second" || final.Snapshot.ActiveProfile != "new" { t.Fatalf("final transition = %#v", final) } if final.Snapshot.Targets[0].State != target.Enabled || final.Snapshot.Targets[1].State != target.Drifted { diff --git a/internal/app/progressive_detection_test.go b/internal/app/progressive_detection_test.go index 33efe43..b4371b6 100644 --- a/internal/app/progressive_detection_test.go +++ b/internal/app/progressive_detection_test.go @@ -60,16 +60,22 @@ func TestApplicationStreamsIndependentTargetDetectionInProductOrder(t *testing.T } } - checking, updates := application.BeginTargetDetection() + stream := application.StartOperation(app.Intent{Kind: app.RefreshTargets}) + checking := stream.Snapshot for _, item := range checking.Targets { if item.DetectionPhase != app.TargetChecking { t.Fatalf("Target did not begin in Checking: %#v", item) } } + updates := stream.Updates + initial := <-updates + if !initial.IsRunning() || len(initial.Progress()) != 3 { + t.Fatalf("initial detection progress = %#v", initial) + } close(secondRelease) second := <-updates - if second.TargetID != "second" || second.Snapshot.Targets[1].DetectionPhase != app.TargetReady || second.Snapshot.Targets[1].State != target.External { + if second.Status().TargetID != "second" || second.Snapshot.Targets[1].DetectionPhase != app.TargetReady || second.Snapshot.Targets[1].State != target.External { t.Fatalf("second completion = %#v", second) } assertOnlyDetectionChanged(t, checking, second.Snapshot, "second") @@ -77,20 +83,20 @@ func TestApplicationStreamsIndependentTargetDetectionInProductOrder(t *testing.T close(failedRelease) failed := <-updates - if failed.TargetID != "failed" || failed.Snapshot.Targets[2].DetectionPhase != app.TargetDetectionFailed || failed.Snapshot.Targets[2].DetectionErr == nil { + if failed.Status().TargetID != "failed" || failed.Snapshot.Targets[2].DetectionPhase != app.TargetDetectionFailed || failed.Snapshot.Targets[2].DetectionErr == nil { t.Fatalf("failed completion = %#v", failed) } assertOnlyDetectionChanged(t, second.Snapshot, failed.Snapshot, "failed") close(firstRelease) first := <-updates - if first.TargetID != "first" || first.Snapshot.Targets[0].DetectionPhase != app.TargetReady || first.Snapshot.Targets[0].State != target.Enabled { + if first.Status().TargetID != "first" || first.Snapshot.Targets[0].DetectionPhase != app.TargetReady || first.Snapshot.Targets[0].State != target.Enabled { t.Fatalf("first completion = %#v", first) } assertOnlyDetectionChanged(t, failed.Snapshot, first.Snapshot, "first") final := <-updates - if !final.Complete { + if !final.IsFailed() { t.Fatalf("final update = %#v", final) } assertTargetOrder(t, final.Snapshot, []string{"first", "second", "failed"}) @@ -127,45 +133,36 @@ func (t *overlappingDetectionTarget) Detect(context.Context, *profile.Profile) t return <-request.result } -func TestOverlappingTargetDetectionDiscardsLateOlderGeneration(t *testing.T) { - adapter := &overlappingDetectionTarget{requests: make(chan detectionRequest, 2)} +func TestOverlappingTargetDetectionIsRejectedAtTheSerializedOperationEntry(t *testing.T) { + adapter := &overlappingDetectionTarget{requests: make(chan detectionRequest, 1)} application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) if err != nil { t.Fatal(err) } - _, olderUpdates := application.BeginTargetDetection() - older := <-adapter.requests - _, newerUpdates := application.BeginTargetDetection() - newer := <-adapter.requests - - newer.result <- target.Detection{ - Availability: target.Availability{Available: true, Evidence: "new availability"}, - Inspection: target.Inspection{State: target.Enabled, Evidence: "new inspection"}, + older := application.StartOperation(app.Intent{Kind: app.RefreshTargets}) + request := <-adapter.requests + newer := application.StartOperation(app.Intent{Kind: app.RefreshTargets}) + closed := <-newer.Updates + if closed.Status().Kind != app.StatusBlocked { + t.Fatalf("overlapping detection = %#v", closed) } - for range newerUpdates { + request.result <- target.Detection{ + Availability: target.Availability{Available: true, Evidence: "authoritative availability"}, + Inspection: target.Inspection{State: target.Enabled, Evidence: "authoritative inspection"}, } - older.result <- target.Detection{ - Availability: target.Availability{Evidence: "old availability"}, - Inspection: target.Inspection{State: target.Disabled, Evidence: "old inspection"}, + for range older.Updates { } - staleUpdates := 0 - for range olderUpdates { - staleUpdates++ - } - if staleUpdates != 0 { - t.Fatalf("older generation published %d stale authoritative update(s)", staleUpdates) - } - got := application.Snapshot().Targets[0] - if !got.Available || got.State != target.Enabled || got.Evidence != "new inspection" { - t.Fatalf("late older generation overwrote newer result: %#v", got) + if !got.Available || got.State != target.Enabled { + t.Fatalf("serialized detection result = %#v", got) } } type mutationInteractionTarget struct { mu sync.Mutex enabled bool + started chan struct{} release chan struct{} } @@ -193,6 +190,7 @@ 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) <-t.release return target.Detection{ Availability: target.Availability{Available: true, Evidence: "stale detection"}, @@ -200,8 +198,8 @@ func (t *mutationInteractionTarget) Detect(context.Context, *profile.Profile) ta } } -func TestDispatchMutationInvalidatesInFlightTargetDetection(t *testing.T) { - adapter := &mutationInteractionTarget{release: make(chan struct{})} +func TestDispatchMutationWaitsForInFlightTargetDetection(t *testing.T) { + adapter := &mutationInteractionTarget{started: make(chan struct{}), release: make(chan struct{})} application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{adapter}) if err != nil { t.Fatal(err) @@ -214,16 +212,21 @@ func TestDispatchMutationInvalidatesInFlightTargetDetection(t *testing.T) { t.Fatal(activated.Err) } - _, updates := application.BeginTargetDetection() - result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: adapter.ID(), Enabled: true}) + detection := application.StartOperation(app.Intent{Kind: app.RefreshTargets}) + <-adapter.started + mutationDone := make(chan app.Result, 1) + go func() { + mutationDone <- application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: adapter.ID(), Enabled: true}) + }() + close(adapter.release) + for range detection.Updates { + } + result := <-mutationDone if result.Err != nil || result.Snapshot.Targets[0].State != target.Enabled { t.Fatalf("mutation result = %#v", result) } - close(adapter.release) - for range updates { - } if got := application.Snapshot().Targets[0]; got.State != target.Enabled || got.Evidence != "mutation completed" { - t.Fatalf("in-flight detection overwrote mutation: %#v", got) + t.Fatalf("serialized mutation state: %#v", got) } } diff --git a/internal/app/target_test.go b/internal/app/target_test.go index cb6507b..155dcab 100644 --- a/internal/app/target_test.go +++ b/internal/app/target_test.go @@ -85,7 +85,7 @@ func TestApplicationRequiresTargetConfirmationBeforeMutation(t *testing.T) { createAndActivate(t, application) warning := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "docker-daemon", Enabled: true}) - if warning.Confirmation == "" || warning.Err != nil || len(adapter.calls) != 0 || warning.Snapshot.Targets[0].State != target.Disabled { + 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}) diff --git a/internal/tui/dashboard_view.go b/internal/tui/dashboard_view.go index 968d2d7..8b55ccb 100644 --- a/internal/tui/dashboard_view.go +++ b/internal/tui/dashboard_view.go @@ -16,9 +16,7 @@ type dashboardRenderState struct { selectedTargetID string focus dashboardPane detailsOffset int - applying bool - profileTransition bool - progressTarget string + operationProgress map[string]app.TargetProgressPhase } func (p terminalPresentation) renderDashboard(state dashboardRenderState, width, height int) string { @@ -51,9 +49,7 @@ func (p terminalPresentation) renderTargetListPane(state dashboardRenderState, w if item.ID == state.selectedTargetID { cursor = "> " } - applying := (state.progressTarget != "" && item.ID == state.progressTarget) || - (!state.profileTransition && state.progressTarget == "" && state.applying && item.ID == state.selectedTargetID) - badge := p.targetBadge(item, applying) + badge := p.targetBadge(item, state.operationProgress[item.ID]) lines = append(lines, cursor+badge+" "+item.Description.Name) } if len(lines) == 0 { @@ -105,7 +101,13 @@ func fixedViewport(content string, width, height, offset int, wrap bool) string return view.View() } -func (p terminalPresentation) targetBadge(item app.TargetSnapshot, applying bool) string { +func (p terminalPresentation) targetBadge(item app.TargetSnapshot, progress app.TargetProgressPhase) string { + switch progress { + case app.TargetProgressRunning: + return p.theme.renderTargetState(item.State, true) + case app.TargetProgressFailed: + return p.theme.checkFailed.Render("! Failed") + } switch item.DetectionPhase { case app.TargetChecking: return p.theme.checking.Render("… Checking") @@ -115,11 +117,11 @@ func (p terminalPresentation) targetBadge(item app.TargetSnapshot, applying bool if !item.Available { return p.theme.unavailable.Render("○ Unavailable") } - return p.theme.renderTargetState(item.State, applying) + return p.theme.renderTargetState(item.State, false) } func (p terminalPresentation) targetDetails(item app.TargetSnapshot) string { - state := p.targetBadge(item, false) + state := p.targetBadge(item, 0) evidence := item.Evidence switch item.DetectionPhase { case app.TargetChecking: diff --git a/internal/tui/foundation_test.go b/internal/tui/foundation_test.go index dbb4b16..f0537e0 100644 --- a/internal/tui/foundation_test.go +++ b/internal/tui/foundation_test.go @@ -267,7 +267,7 @@ func responsiveLayoutModule() *recordingModule { }} } -func resizedModel(t *testing.T, module app.Module, mode themeMode, width, height int) Model { +func resizedModel(t *testing.T, module app.InteractiveModule, mode themeMode, width, height int) Model { t.Helper() return resize(NewModel(module, withTheme(mode)), width, height) } diff --git a/internal/tui/model.go b/internal/tui/model.go index 5efb828..d3bab47 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -31,64 +31,22 @@ type formState struct { } type Model struct { - application app.Module - presentation terminalPresentation - snapshot app.Snapshot - page page - width int - height int - selected int - selectedTargetID string - dashboardFocus dashboardPane - showUnavailable bool - detailsOffset int - form formState - confirm bool - targetConfirm *app.Intent - profileConfirm *app.Intent - pendingActivation app.Intent - status string - operationStatus app.OperationStatus - currentError *app.OperationStatus - errorExpanded bool - confirmation *app.Confirmation - operationActive bool - operationKind app.IntentKind - operationIntent app.Intent - operationUpdates <-chan app.OperationUpdate - operationProgress map[string]app.TargetProgressPhase - applying bool - refreshing bool - detectionUpdates <-chan app.TargetDetectionUpdate - profileTransition bool - progressTarget string - quitRequested bool - detect func(context.Context, string) (probe.Capabilities, error) -} - -type targetOperationDone struct { - result app.Result -} - -type refreshDone struct { - result app.Result -} - -type targetDetectionProgress struct { - update app.TargetDetectionUpdate - updates <-chan app.TargetDetectionUpdate - closed bool -} - -type profileOperationProgress struct { - result app.Result - updates <-chan app.Result -} - -type operationProgress struct { - update app.OperationUpdate - updates <-chan app.OperationUpdate - closed bool + application app.InteractiveModule + presentation terminalPresentation + snapshot app.Snapshot + page page + width int + height int + selected int + selectedTargetID string + dashboardFocus dashboardPane + showUnavailable bool + detailsOffset int + form formState + confirm bool + status string + operation operationCoordinator + detect func(context.Context, string) (probe.Capabilities, error) } // probeDone carries one finished Protocol Probe back to the form. address is @@ -102,54 +60,23 @@ type probeDone struct { gen uint64 } -func waitForTargetDetection(updates <-chan app.TargetDetectionUpdate) tea.Cmd { - return func() tea.Msg { - update, ok := <-updates - if !ok { - return targetDetectionProgress{updates: updates, closed: true} - } - return targetDetectionProgress{update: update, updates: updates} - } -} - -func waitForOperation(updates <-chan app.OperationUpdate) tea.Cmd { - return func() tea.Msg { - update, ok := <-updates - if !ok { - return operationProgress{updates: updates, closed: true} - } - return operationProgress{update: update, updates: updates} - } -} - -func waitForProfileProgress(updates <-chan app.Result) tea.Cmd { - return func() tea.Msg { - result, ok := <-updates - if !ok { - return profileOperationProgress{result: app.Result{Complete: true}, updates: updates} - } - return profileOperationProgress{result: result, updates: updates} - } -} - -func NewModel(application app.Module, configure ...ModelOption) Model { +func NewModel(application app.InteractiveModule, configure ...ModelOption) Model { options := defaultModelOptions() for _, option := range configure { option(&options) } snapshot := application.Snapshot() model := Model{ - application: application, - presentation: terminalPresentation{theme: newSemanticTheme(options.theme, options.output)}, - snapshot: snapshot, - selectedTargetID: newTargetCatalog(snapshot, false, "").firstID(), - width: 80, - height: 24, - operationProgress: make(map[string]app.TargetProgressPhase), - detect: probe.Detect, + application: application, + presentation: terminalPresentation{theme: newSemanticTheme(options.theme, options.output)}, + snapshot: snapshot, + selectedTargetID: newTargetCatalog(snapshot, false, "").firstID(), + width: 80, + height: 24, + operation: newOperationCoordinator(), + detect: probe.Detect, } - _, progressiveDetection := application.(app.ProgressiveDetectionModule) - if targetsChecking(snapshot) || progressiveDetection { + if targetsChecking(snapshot) { model.startOperation(app.Intent{Kind: app.RefreshTargets}) } return model @@ -165,8 +92,8 @@ func targetsChecking(snapshot app.Snapshot) bool { } func (m Model) Init() tea.Cmd { - if m.operationUpdates != nil { - return waitForOperation(m.operationUpdates) + if m.operation.updates != nil { + return waitForOperation(m.operation.updates) } return nil } @@ -174,16 +101,14 @@ func (m Model) Init() tea.Cmd { // busy is the single-operation guard for asynchronous Application work. Every // operation start checks it, so a completion can only belong to the one current // operation rather than racing another operation for shared model state. -func (m Model) busy() bool { - return m.quitRequested || m.operationInFlight() || m.pendingActivation.Kind != 0 -} +func (m Model) busy() bool { return m.operation.busy() } -func (m Model) operationInFlight() bool { - return m.operationActive || m.applying || m.refreshing || m.profileTransition -} +func (m Model) operationInFlight() bool { return m.operation.active } func (m Model) dashboardNavigationAvailable() bool { - return !m.quitRequested + // A deferred exit rejects new work, not read-only browsing. Selection, + // details, and Help remain useful until the current mutation is safe. + return true } func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { @@ -197,139 +122,41 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { // been consumed. Until Bubble Tea processes that QuitMsg, keep the model // terminal so intervening input cannot cancel a confirmation or start a // second mutation. - if m.quitRequested && !m.operationInFlight() { + if m.operation.quitRequested && !m.operationInFlight() { return m, nil } if key, ok := message.(tea.KeyMsg); ok { - exitRequest := key.String() == "ctrl+c" || (m.page == mainPage && key.String() == "q" && m.targetConfirm == nil && m.profileConfirm == nil) + // Dashboard q always means exit, even when confirmation is visible. + // Escape remains the deterministic cancel/back key. + exitRequest := key.String() == "ctrl+c" || (m.page == mainPage && key.String() == "q") if exitRequest { - if m.operationInFlight() { - m.quitRequested = true - return m, nil - } - return m, tea.Quit + return m, m.operation.requestQuit() } } if progress, ok := message.(operationProgress); ok { - if !m.operationActive || progress.updates != m.operationUpdates { + if !m.operation.active || progress.updates != m.operation.updates { return m, nil } if progress.closed { m.snapshot = m.application.Snapshot() m.retainTargetSelection() - m.finishOperation() - return m, m.deferredQuit() + m.operation.unexpectedClose() + return m, m.operation.deferredQuit() } - m.applyOperationUpdate(progress.update) - if isTerminalOperationPhase(progress.update.Phase) { - m.finishOperation() - return m, m.deferredQuit() + m.snapshot = progress.update.Snapshot + m.retainTargetSelection() + if firstFailed := m.operation.apply(progress.update); firstFailed != "" { + m.selectedTargetID = firstFailed + m.detailsOffset = 0 + } + if m.operation.confirmation != nil { + m.page = mainPage + } + if progress.update.IsTerminal() { + return m, m.operation.deferredQuit() } return m, waitForOperation(progress.updates) } - if progress, ok := message.(targetDetectionProgress); ok { - if !m.refreshing || progress.updates != m.detectionUpdates { - return m, nil - } - if progress.closed { - // A stale Application generation closes without claiming an - // authoritative completion snapshot. Reconcile with the Application - // so this Model does not strand rows in its superseded Checking frame. - formerCatalog := newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID) - formerIndex := formerCatalog.index(m.selectedTargetID) - m.snapshot = m.application.Snapshot() - m.selectedTargetID = newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID).retain(m.selectedTargetID, formerIndex) - m.refreshing = false - m.detectionUpdates = nil - return m, m.deferredQuit() - } - formerCatalog := newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID) - formerIndex := formerCatalog.index(m.selectedTargetID) - m.snapshot = progress.update.Snapshot - m.selectedTargetID = newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID).retain(m.selectedTargetID, formerIndex) - if progress.update.Complete { - m.refreshing = false - m.detectionUpdates = nil - m.status = "Target availability and state checks completed" - return m, m.deferredQuit() - } - return m, waitForTargetDetection(progress.updates) - } - if completed, ok := message.(refreshDone); ok { - if !m.refreshing { - return m, nil - } - m.refreshing = false - formerCatalog := newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID) - formerIndex := formerCatalog.index(m.selectedTargetID) - m.snapshot = completed.result.Snapshot - m.selectedTargetID = newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID).retain(m.selectedTargetID, formerIndex) - if completed.result.Err != nil { - m.status = "Error: " + completed.result.Err.Error() - } else { - m.status = completed.result.Notice - } - return m, m.deferredQuit() - } - if progress, ok := message.(profileOperationProgress); ok { - if !m.profileTransition { - return m, nil - } - m.snapshot = progress.result.Snapshot - m.retainTargetSelection() - m.progressTarget = "" - if len(progress.result.Progress) > 0 { - item := progress.result.Progress[len(progress.result.Progress)-1] - if item.Applying { - m.progressTarget = item.TargetID - } - } - if progress.result.Confirmation != "" { - m.applying = false - m.profileTransition = false - m.page = profilesPage - intent := m.pendingActivation - m.profileConfirm = &intent - m.status = progress.result.Confirmation - return m, m.deferredQuit() - } - if progress.result.Err != nil { - m.status = "Error: " + progress.result.Err.Error() - } else if progress.result.Notice != "" { - m.status = progress.result.Notice - } - if progress.result.Complete { - m.applying = false - m.profileTransition = false - m.progressTarget = "" - m.pendingActivation = app.Intent{} - return m, m.deferredQuit() - } - return m, waitForProfileProgress(progress.updates) - } - if completed, ok := message.(targetOperationDone); ok { - if !m.applying || m.profileTransition { - return m, nil - } - m.applying = false - m.snapshot = completed.result.Snapshot - m.retainTargetSelection() - if completed.result.Confirmation != "" { - intent := app.Intent{Kind: app.SetTargetDesired} - if m.targetConfirm != nil { - intent = *m.targetConfirm - } - m.targetConfirm = &intent - m.status = completed.result.Confirmation - } else if completed.result.Err != nil { - m.targetConfirm = nil - m.status = "Error: " + completed.result.Err.Error() - } else { - m.targetConfirm = nil - m.status = completed.result.Notice - } - return m, m.deferredQuit() - } if done, ok := message.(probeDone); ok { if !m.form.probing || done.gen != m.form.probeGen { return m, nil @@ -396,172 +223,93 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { } func (m *Model) startOperation(intent app.Intent) tea.Cmd { - if m.quitRequested { - return nil + snapshot, command, started := m.operation.start(m.application, intent) + if started { + m.snapshot = snapshot + m.retainTargetSelection() } - if m.operationActive { - m.operationStatus = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before starting another modification"} - return nil - } - interactive, ok := m.application.(app.InteractiveModule) - if !ok { - m.operationStatus = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Interactive operation streaming is unavailable"} - return nil - } - stream := interactive.StartOperation(intent) - m.snapshot = stream.Snapshot - m.retainTargetSelection() - m.operationActive = true - m.operationKind = intent.Kind - m.operationIntent = intent - m.operationUpdates = stream.Updates - m.operationProgress = make(map[string]app.TargetProgressPhase) - m.applying = intent.Kind == app.SetTargetDesired || intent.Kind == app.ActivateProfile - m.refreshing = intent.Kind == app.RefreshTargets - m.profileTransition = intent.Kind == app.ActivateProfile - m.progressTarget = "" - m.operationStatus = app.OperationStatus{Kind: app.StatusProgress, Summary: operationStartingSummary(intent.Kind)} - return waitForOperation(stream.Updates) + return command } -func operationStartingSummary(kind app.IntentKind) string { - switch kind { - case app.RefreshTargets: - return "Checking Target availability and state…" - case app.ActivateProfile: - return "Changing Active Profile…" +type mutationAvailability struct { + available bool + reason string + 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 { + 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"} default: - return "Starting Target mutation…" + return mutationAvailability{available: true} } } -func (m *Model) finishOperation() { - m.operationActive = false - m.operationKind = 0 - m.operationIntent = app.Intent{} - m.operationUpdates = nil - m.applying = false - m.refreshing = false - m.profileTransition = false - m.progressTarget = "" -} - -func isTerminalOperationPhase(phase app.OperationPhase) bool { - return phase == app.OperationAwaitingConfirmation || phase == app.OperationSucceeded || phase == app.OperationFailed || phase == app.OperationBlocked -} - -func (m *Model) applyOperationUpdate(update app.OperationUpdate) { - m.snapshot = update.Snapshot - m.retainTargetSelection() - for _, progress := range update.Progress { - m.operationProgress[progress.TargetID] = progress.Phase - if progress.Phase == app.TargetProgressRunning { - m.progressTarget = progress.TargetID - } else if m.progressTarget == progress.TargetID { - m.progressTarget = "" - } - } - if update.Status.Kind != 0 { - m.operationStatus = update.Status - } - if update.Status.Kind == app.StatusError { - current := update.Status - if current.Detail == "" && update.Err != nil { - current.Detail = update.Err.Error() - } - m.currentError = ¤t - m.errorExpanded = false - } else if update.Phase == app.OperationSucceeded && m.currentError != nil && update.Status.TargetID != "" && update.Status.TargetID == m.currentError.TargetID { - m.currentError = nil - m.errorExpanded = false - } - if update.FirstFailedTargetID != "" { - m.selectedTargetID = update.FirstFailedTargetID - m.detailsOffset = 0 - } - if isTerminalOperationPhase(update.Phase) && update.Phase != app.OperationAwaitingConfirmation { - m.pendingActivation = app.Intent{} - } - if update.Phase == app.OperationAwaitingConfirmation && update.Confirmation != nil { - intent := m.operationIntent - m.confirmation = update.Confirmation - m.page = mainPage - if intent.Kind == app.ActivateProfile { - m.pendingActivation = intent - m.profileConfirm = &intent - } else { - m.targetConfirm = &intent - } - } -} - -func isQuitKey(key tea.KeyMsg) bool { - switch key.String() { - case "q", "esc", "ctrl+c": - return true - default: - return false - } -} - -func (m Model) deferredQuit() tea.Cmd { - if m.quitRequested { - return tea.Quit - } - return nil -} - func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { - if m.targetConfirm != nil || m.profileConfirm != nil { - intent := m.targetConfirm - if intent == nil { - intent = m.profileConfirm - } + if m.operation.confirmation != nil { switch key.String() { case "y": - confirmed := *intent - confirmed.Confirm = true - m.targetConfirm = nil - m.profileConfirm = nil - m.pendingActivation = app.Intent{} - m.confirmation = nil + confirmed, ok := m.operation.confirmedIntent() + if !ok { + return m, nil + } m.status = "" return m, m.startOperation(confirmed) - case "enter", "n", "esc", "q": - m.targetConfirm = nil - m.profileConfirm = nil - m.pendingActivation = app.Intent{} - m.confirmation = nil - m.operationStatus = app.OperationStatus{} + case "enter", "n", "esc": + m.operation.cancelConfirmation() } return m, nil } - if key.String() == "e" && m.currentError != nil { - m.errorExpanded = !m.errorExpanded + if key.String() == "e" && m.operation.currentError != nil { + m.operation.errorExpanded = !m.operation.errorExpanded + m.operation.errorOffset = 0 return m, nil } - if key.String() == "x" && m.currentError != nil { - m.currentError = nil - m.errorExpanded = false + if key.String() == "x" && m.operation.currentError != nil { + m.operation.acknowledgeError() return m, nil } + if m.operation.errorExpanded { + switch key.String() { + case "pgup": + m.operation.errorOffset = max(0, m.operation.errorOffset-5) + return m, nil + case "pgdown": + m.operation.errorOffset += 5 + return m, nil + } + } catalog := newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID) switch key.String() { case "p": - if m.operationActive || m.quitRequested { - m.operationStatus = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before opening Profile management"} + if m.operation.active || m.operation.quitRequested { + m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before opening Profile management"} } else { m.page = profilesPage m.selected = 0 m.status = "" + if m.operation.status.Kind != app.StatusError { + m.operation.status = app.OperationStatus{} + } } case "?": - if !m.quitRequested { - m.page = helpPage - } + m.page = helpPage case "r": - if m.operationActive || m.quitRequested { - m.operationStatus = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before refreshing Targets"} + if m.operation.active || m.operation.quitRequested { + m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before refreshing Targets"} } else { return m, m.startOperation(app.Intent{Kind: app.RefreshTargets}) } @@ -607,41 +355,21 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { m.detailsOffset += 5 } case " ": - if m.operationActive || m.quitRequested { - m.operationStatus = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before starting another modification"} - break - } selected, ok := catalog.selected(m.selectedTargetID) - if !ok || !selected.Available || selected.DetectionPhase != app.TargetReady { + availability := targetMutationAvailability(selected, ok, m.snapshot.ActiveProfile, 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} return m, m.startOperation(intent) - case "esc": - if !m.operationActive { - return m, tea.Quit - } } + return m, nil } func (m Model) updateProfiles(key tea.KeyMsg) (tea.Model, tea.Cmd) { - if m.profileConfirm != nil { - switch key.String() { - case "y": - intent := *m.profileConfirm - intent.Confirm = true - m.profileConfirm = nil - m.pendingActivation = app.Intent{} - return m.beginProfileActivation(intent) - case "n", "esc", "q": - m.profileConfirm = nil - m.pendingActivation = app.Intent{} - m.status = "" - } - return m, nil - } if m.confirm { switch key.String() { case "y": @@ -778,10 +506,9 @@ func (m Model) updateForm(key tea.KeyMsg) (tea.Model, tea.Cmd) { func (m Model) beginProfileActivation(intent app.Intent) (tea.Model, tea.Cmd) { if m.busy() { - m.operationStatus = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before changing the Active Profile"} + m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before changing the Active Profile"} return m, nil } - m.pendingActivation = intent m.page = mainPage m.selected = 0 return m, m.startOperation(intent) @@ -790,11 +517,6 @@ func (m Model) beginProfileActivation(intent app.Intent) (tea.Model, tea.Cmd) { func (m *Model) apply(intent app.Intent) { result := m.application.Dispatch(intent) m.snapshot = result.Snapshot - if result.Confirmation != "" { - m.profileConfirm = &intent - m.status = result.Confirmation - return - } if m.selected >= len(m.snapshot.Profiles) && m.selected > 0 { m.selected-- } @@ -842,19 +564,15 @@ func (m Model) View() string { detailsOffset: m.detailsOffset, form: m.form, confirm: m.confirm, - targetConfirm: m.targetConfirm != nil, - profileConfirm: m.profileConfirm != nil, - confirmation: m.confirmation, + confirmation: m.operation.confirmation, status: m.status, - operationStatus: m.operationStatus, - currentError: m.currentError, - errorExpanded: m.errorExpanded, - exitDeferred: m.quitRequested && m.operationInFlight(), - operationActive: m.operationActive, - operationKind: m.operationKind, - applying: m.applying, - profileTransition: m.profileTransition, - progressTarget: m.progressTarget, + 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/model_test.go b/internal/tui/model_test.go index 9980735..731bcf1 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -26,13 +26,12 @@ func (m *recordingModule) Dispatch(intent app.Intent) app.Result { func (m *recordingModule) StartOperation(intent app.Intent) app.OperationStream { result := m.Dispatch(intent) updates := make(chan app.OperationUpdate, 1) - phase := app.OperationSucceeded - status := app.OperationStatus{Kind: app.StatusSuccess, Summary: result.Notice} + status := app.OperationStatus{Summary: result.Notice} if result.Err != nil { - phase = app.OperationFailed - status = app.OperationStatus{Kind: app.StatusError, Summary: "Operation failed", Detail: result.Err.Error()} + updates <- app.FailedOperationUpdate(result.Snapshot, app.OperationStatus{Summary: "Operation failed"}, nil, "", result.Err) + } else { + updates <- app.SucceededOperationUpdate(result.Snapshot, status, nil) } - updates <- app.OperationUpdate{Snapshot: result.Snapshot, Phase: phase, Status: status, Err: result.Err} close(updates) return app.OperationStream{Snapshot: m.snapshot, Updates: updates} } diff --git a/internal/tui/operation_coordination_test.go b/internal/tui/operation_coordination_test.go index 8ee41b7..c6625f2 100644 --- a/internal/tui/operation_coordination_test.go +++ b/internal/tui/operation_coordination_test.go @@ -57,9 +57,9 @@ func TestCompleteModelKeepsDashboardAndHelpBrowsableWhileMutationIsSerialized(t updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - updates <- app.OperationUpdate{Snapshot: module.snapshot, Phase: app.OperationRunning, - Status: app.OperationStatus{Kind: app.StatusProgress, Summary: "Writing Shell configuration", TargetID: "shell"}, - Progress: []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressRunning}}} + updates <- app.RunningOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Writing Shell configuration", TargetID: "shell"}, + []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressRunning}}) model, command = applyOperationCommand(t, model, command) assertUsableFrame(t, model.View(), 120, 36, "Applying", "Writing Shell configuration") @@ -96,9 +96,10 @@ func TestCompleteModelPreservesExpandableCurrentErrorAndFocusesFirstFailedTarget failedSnapshot := operationSnapshot() failedSnapshot.Targets[0].State = target.Enabled failedSnapshot.Targets[1].State = target.Drifted - failure <- app.OperationUpdate{Snapshot: failedSnapshot, Phase: app.OperationFailed, FirstFailedTargetID: "apt", Err: errors.New("apt write failed: permission denied"), - Status: app.OperationStatus{Kind: app.StatusError, Summary: "Active Profile changed with one Target failure", Detail: "apt write failed: permission denied", TargetID: "apt"}, - Progress: []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressSucceeded}, {TargetID: "apt", Phase: app.TargetProgressFailed}}} + failure <- app.FailedOperationUpdate(failedSnapshot, + app.OperationStatus{Summary: "Active Profile changed with one Target failure", TargetID: "apt"}, + []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressSucceeded}, {TargetID: "apt", Phase: app.TargetProgressFailed}}, + "apt", errors.New("apt write failed: permission denied")) close(failure) model, _ = applyOperationCommand(t, model, command) if model.selectedTargetID != "apt" { @@ -115,19 +116,57 @@ func TestCompleteModelPreservesExpandableCurrentErrorAndFocusesFirstFailedTarget updated, command = model.Update(keyMessage("r")) model = updated.(Model) - refresh <- app.OperationUpdate{Snapshot: failedSnapshot, Phase: app.OperationSucceeded, - Status: app.OperationStatus{Kind: app.StatusSuccess, Summary: "Target checks completed"}} + refresh <- app.SucceededOperationUpdate(failedSnapshot, + app.OperationStatus{Summary: "Shell Target check completed", TargetID: "shell"}, nil) close(refresh) model, _ = applyOperationCommand(t, model, command) if !strings.Contains(model.View(), "Current error: Active Profile changed with one Target failure") { t.Fatalf("unrelated success replaced current error:\n%s", model.View()) } model = updateKey(model, "x") - if strings.Contains(model.View(), "Current error:") { + if strings.Contains(model.View(), "Current error:") || strings.Contains(model.View(), "Active Profile changed with one Target failure") { t.Fatalf("acknowledged error remained visible:\n%s", model.View()) } } +func TestCompleteModelClearsCurrentErrorForDirectlyRelatedSuccessfulDetection(t *testing.T) { + failure := make(chan app.OperationUpdate, 1) + resolution := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{failure, resolution}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + failure <- app.FailedOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Shell Target update failed", TargetID: "shell"}, nil, "shell", errors.New("permission denied")) + close(failure) + model, _ = applyOperationCommand(t, model, command) + + updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + resolution <- app.SucceededOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Shell Target check completed", TargetID: "shell"}, + []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressSucceeded}}) + close(resolution) + model, _ = applyOperationCommand(t, model, command) + if strings.Contains(model.View(), "Current error:") { + t.Fatalf("explicit related resolution did not clear the current error:\n%s", model.View()) + } +} + +func TestCompleteModelReportsOperationStreamClosingWithoutFinalResult(t *testing.T) { + updates := make(chan app.OperationUpdate) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{updates}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + close(updates) + model, _ = applyOperationCommand(t, model, command) + if !strings.Contains(model.View(), "Current error: Operation ended without a final result") || model.operation.active { + t.Fatalf("unexpected stream closure was not deterministic:\n%s", model.View()) + } +} + func TestCompleteModelUsesStructuredConfirmationAndSafeCancellation(t *testing.T) { confirmation := make(chan app.OperationUpdate, 1) module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{confirmation}} @@ -135,9 +174,8 @@ func TestCompleteModelUsesStructuredConfirmationAndSafeCancellation(t *testing.T updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - confirmation <- app.OperationUpdate{Snapshot: module.snapshot, Phase: app.OperationAwaitingConfirmation, - Status: app.OperationStatus{Kind: app.StatusConfirmation, Summary: "Confirm Docker restart"}, - Confirmation: &app.Confirmation{Title: "Confirm Docker restart", Summary: "Running work may be disrupted.", ConfirmLabel: "Restart", CancelLabel: "Cancel", HighImpact: true}} + confirmation <- app.ConfirmationOperationUpdate(module.snapshot, + app.Confirmation{Title: "Confirm Docker restart", Summary: "Running work may be disrupted.", ConfirmLabel: "Restart", CancelLabel: "Cancel", HighImpact: true}) close(confirmation) model, _ = applyOperationCommand(t, model, command) view := model.View() @@ -145,8 +183,29 @@ func TestCompleteModelUsesStructuredConfirmationAndSafeCancellation(t *testing.T updated, next := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) model = updated.(Model) - if next != nil || model.targetConfirm != nil || len(module.intents) != 1 { - t.Fatalf("safe-default Enter did not cancel: confirmation=%v intents=%#v", model.targetConfirm != nil, module.intents) + if next != nil || model.operation.confirmation != nil || len(module.intents) != 1 { + t.Fatalf("safe-default Enter did not cancel: confirmation=%v intents=%#v", model.operation.confirmation != nil, module.intents) + } +} + +func TestCompleteModelExplainsDisabledTargetMutations(t *testing.T) { + checking := operationSnapshot() + checking.ActiveProfile = "" + module := &interactiveScriptModule{snapshot: checking} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + model.snapshot.Targets[0].DetectionPhase = app.TargetChecking + + 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()) + } + + model.snapshot.Targets[0].DetectionPhase = app.TargetReady + 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") { + t.Fatalf("missing-Profile mutation lacked a visible reason:\n%s", model.View()) } } @@ -163,6 +222,16 @@ func TestCompleteModelVisiblyDefersCtrlCFromHelpAndStartsNoNewWork(t *testing.T) if quit != nil || !strings.Contains(model.View(), "Exit requested — waiting for the current mutation to finish safely") { t.Fatalf("deferred exit was not visible from Help:\n%s", model.View()) } + model = updateKey(model, "q") + model = updateKey(model, "down") + if model.selectedTargetID != "apt" { + t.Fatalf("deferred exit prevented read-only Target browsing: selected=%q", model.selectedTargetID) + } + model = updateKey(model, "?") + if model.page != helpPage { + t.Fatal("deferred exit prevented opening read-only Help") + } + model = updateKey(model, "q") for _, key := range []tea.KeyMsg{tea.KeyMsg{Type: tea.KeySpace}, keyMessage("r"), keyMessage("p")} { updated, command := model.Update(key) model = updated.(Model) @@ -171,8 +240,9 @@ func TestCompleteModelVisiblyDefersCtrlCFromHelpAndStartsNoNewWork(t *testing.T) } } - updates <- app.OperationUpdate{Snapshot: module.snapshot, Phase: app.OperationSucceeded, - Status: app.OperationStatus{Kind: app.StatusSuccess, Summary: "Shell Target is now Enabled", TargetID: "shell"}} + updates <- app.SucceededOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Shell Target is now Enabled", TargetID: "shell"}, + []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressSucceeded}}) close(updates) model, quit = applyOperationCommand(t, model, operation) if quit == nil { diff --git a/internal/tui/operation_coordinator.go b/internal/tui/operation_coordinator.go new file mode 100644 index 0000000..6b3b2b5 --- /dev/null +++ b/internal/tui/operation_coordinator.go @@ -0,0 +1,198 @@ +package tui + +import ( + "errors" + + tea "github.com/charmbracelet/bubbletea" + + "proxyctl/internal/app" +) + +type operationError struct { + status app.OperationStatus + detail string +} + +// operationCoordinator owns the complete interactive operation lifecycle. The +// root Model delegates stream consumption, progress, confirmation, current +// error, and deferred-exit state here instead of mirroring those states. +type operationCoordinator struct { + active bool + intent app.Intent + updates <-chan app.OperationUpdate + progress map[string]app.TargetProgressPhase + status app.OperationStatus + currentError *operationError + errorExpanded bool + errorOffset int + confirmation *app.Confirmation + confirmationIntent *app.Intent + quitRequested bool +} + +type operationProgress struct { + update app.OperationUpdate + updates <-chan app.OperationUpdate + closed bool +} + +func newOperationCoordinator() operationCoordinator { + return operationCoordinator{progress: make(map[string]app.TargetProgressPhase)} +} + +func waitForOperation(updates <-chan app.OperationUpdate) tea.Cmd { + return func() tea.Msg { + update, ok := <-updates + if !ok { + return operationProgress{updates: updates, closed: true} + } + return operationProgress{update: update, updates: updates} + } +} + +func (o *operationCoordinator) start(application app.InteractiveModule, intent app.Intent) (app.Snapshot, tea.Cmd, bool) { + if o.quitRequested || o.active || o.confirmation != nil { + o.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current operation to finish before starting another modification"} + return application.Snapshot(), nil, false + } + stream := application.StartOperation(intent) + o.active = true + o.intent = intent + o.updates = stream.Updates + o.progress = make(map[string]app.TargetProgressPhase) + if intent.TargetID != "" { + o.progress[intent.TargetID] = app.TargetProgressRunning + } + o.status = app.OperationStatus{Kind: app.StatusProgress, Summary: operationStartingSummary(intent.Kind)} + return stream.Snapshot, waitForOperation(stream.Updates), true +} + +func operationStartingSummary(kind app.IntentKind) string { + switch kind { + case app.RefreshTargets: + return "Checking Target availability and state…" + case app.ActivateProfile: + return "Changing Active Profile…" + default: + return "Starting Target mutation…" + } +} + +func (o *operationCoordinator) apply(update app.OperationUpdate) (firstFailedTargetID string) { + status := update.Status() + progress := update.Progress() + for _, item := range progress { + o.progress[item.TargetID] = item.Phase + if item.Phase == app.TargetProgressSucceeded { + o.resolveTargetError(item.TargetID) + } + } + if status.Summary != "" { + o.status = status + } + if update.IsSucceeded() { + o.resolveTargetError(status.TargetID) + } + if update.IsFailed() { + o.currentError = &operationError{status: status, detail: errorDetail(update.Err())} + o.errorExpanded = false + o.errorOffset = 0 + firstFailedTargetID = update.FirstFailedTargetID() + } + if request := update.Confirmation(); request != nil { + intent := o.intent + o.confirmation = request + o.confirmationIntent = &intent + } + if update.IsTerminal() { + o.finish() + } + return firstFailedTargetID +} + +func errorDetail(err error) string { + if err == nil { + return "" + } + return err.Error() +} + +func (o *operationCoordinator) resolveTargetError(targetID string) { + if targetID == "" || o.currentError == nil || o.currentError.status.TargetID != targetID { + return + } + o.currentError = nil + o.errorExpanded = false + o.errorOffset = 0 +} + +func (o *operationCoordinator) finish() { + o.active = false + o.intent = app.Intent{} + o.updates = nil +} + +func (o *operationCoordinator) unexpectedClose() { + o.status = app.OperationStatus{Kind: app.StatusError, Summary: "Operation ended without a final result"} + o.currentError = &operationError{ + status: o.status, + detail: "The Application operation stream closed before publishing a terminal result.", + } + o.errorExpanded = false + o.errorOffset = 0 + o.finish() +} + +func (o *operationCoordinator) acknowledgeError() { + o.currentError = nil + o.errorExpanded = false + o.errorOffset = 0 + if o.status.Kind == app.StatusError { + o.status = app.OperationStatus{} + } +} + +func (o *operationCoordinator) cancelConfirmation() { + o.confirmation = nil + o.confirmationIntent = nil + o.status = app.OperationStatus{} +} + +func (o *operationCoordinator) confirmedIntent() (app.Intent, bool) { + if o.confirmationIntent == nil { + return app.Intent{}, false + } + intent := *o.confirmationIntent + intent.Confirm = true + o.confirmation = nil + o.confirmationIntent = nil + return intent, true +} + +func (o operationCoordinator) busy() bool { + return o.quitRequested || o.active || o.confirmation != nil +} + +func (o operationCoordinator) deferredQuit() tea.Cmd { + if o.quitRequested && !o.active { + return tea.Quit + } + return nil +} + +func (o *operationCoordinator) requestQuit() tea.Cmd { + o.quitRequested = true + if o.active { + return nil + } + return tea.Quit +} + +func (o *operationCoordinator) fail(err error, summary string) { + if err == nil { + err = errors.New(summary) + } + o.status = app.OperationStatus{Kind: app.StatusError, Summary: summary} + o.currentError = &operationError{status: o.status, detail: err.Error()} + o.finish() +} diff --git a/internal/tui/pages.go b/internal/tui/pages.go index 873c099..97f6910 100644 --- a/internal/tui/pages.go +++ b/internal/tui/pages.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "proxyctl/internal/app" "proxyctl/internal/profile" ) @@ -22,11 +21,14 @@ func (p terminalPresentation) mainView(model presentationState, width, height in catalog := newTargetCatalog(model.snapshot, model.showUnavailable, model.selectedTargetID) footer := p.dashboardFooter(model, catalog) - if model.targetConfirm && !model.operationActive { - footer = "y: confirm Enter/n/Esc: Cancel" + if model.confirmation != nil && !model.operationActive { + footer = "y: confirm Enter/n/Esc: Cancel q: exit" } if model.currentError != nil { footer += "\ne: expand/collapse current error x: acknowledge error" + if model.errorExpanded { + footer += " PgUp/PgDn: scroll error details" + } } page := terminalPage{ header: header, @@ -36,9 +38,7 @@ func (p terminalPresentation) mainView(model presentationState, width, height in selectedTargetID: model.selectedTargetID, focus: model.dashboardFocus, detailsOffset: model.detailsOffset, - applying: model.applying, - profileTransition: model.profileTransition, - progressTarget: model.progressTarget, + operationProgress: model.operationProgress, }, } if model.confirmation != nil { @@ -66,21 +66,21 @@ func (p terminalPresentation) dashboardFooter(model presentationState, catalog t visibility = "u: hide unavailable" } if model.dashboardFocus == targetDetailsPane { - footer := "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets " + visibility + "\np: manage Proxy Profiles ?: help q/Esc: quit" + footer := "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets " + visibility + "\np: manage Proxy Profiles ?: help q: quit" if model.operationActive { footer = "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets ?: help\nMutations disabled: wait for the current mutation to finish" } return footer } actions := "↑/k ↓/j: select Tab: focus details " + visibility - if selected, ok := catalog.selected(model.selectedTargetID); ok && selected.Available && selected.DetectionPhase == app.TargetReady { - if model.operationActive { - actions = "Space: disabled (mutation in progress) " + actions - } else { - actions = "Space: toggle " + actions - } + selected, ok := catalog.selected(model.selectedTargetID) + availability := targetMutationAvailability(selected, ok, model.snapshot.ActiveProfile, model.operationActive || model.exitDeferred) + if availability.available { + actions = "Space: toggle " + actions + } else { + actions = "Space: disabled (" + availability.compactReason + ") " + actions } - footer := actions + "\np: manage Proxy Profiles r: refresh ?: help q/Esc: quit" + footer := actions + "\np: manage Proxy Profiles r: refresh ?: help q: quit" if model.operationActive { footer = actions + "\n?: help q: defer safe exit; new mutations disabled" } @@ -127,8 +127,6 @@ func (p terminalPresentation) profilesView(model presentationState) terminalPage context := selectedProfileDetails(p, model) footer := "n: new e: edit d: delete a/Enter: activate\n↑/k ↓/j: select q/Esc: back" switch { - case model.profileConfirm: - footer = "y: confirm n/Esc: cancel" case model.confirm: footer = "y: confirm n/Esc: cancel" if len(model.snapshot.Profiles) == 1 { diff --git a/internal/tui/presentation.go b/internal/tui/presentation.go index 801fa5d..228f8cb 100644 --- a/internal/tui/presentation.go +++ b/internal/tui/presentation.go @@ -64,19 +64,15 @@ type presentationState struct { detailsOffset int form formState confirm bool - targetConfirm bool - profileConfirm bool confirmation *app.Confirmation status string operationStatus app.OperationStatus - currentError *app.OperationStatus + currentError *operationError errorExpanded bool + errorOffset int exitDeferred bool operationActive bool - operationKind app.IntentKind - applying bool - profileTransition bool - progressTarget string + operationProgress map[string]app.TargetProgressPhase } // terminalPresentation is the terminal-presentation Module. Application @@ -88,13 +84,15 @@ type terminalPresentation struct { } type terminalPage struct { - header string - primary string - context string - footer string - status string - focusLine int - dashboard *dashboardRenderState + header string + primary string + context string + footer string + status string + statusExpanded bool + statusOffset int + focusLine int + dashboard *dashboardRenderState } func (p terminalPresentation) Render(state presentationState) string { @@ -123,6 +121,8 @@ func (p terminalPresentation) Render(state presentationState) string { page = p.mainView(state, width, height) } page.status = renderStatus(state) + page.statusExpanded = state.errorExpanded && state.currentError != nil + page.statusOffset = state.errorOffset page = redactTerminalPage(page) // Dashboard composition retains structured Target metadata until pane sizes // are known. Redact the fully composed frame as the final presentation @@ -140,9 +140,9 @@ func renderStatus(state presentationState) string { lines = append(lines, state.status) } if state.currentError != nil { - errorText := "Current error: " + state.currentError.Summary - if state.errorExpanded && state.currentError.Detail != "" { - errorText += " — " + state.currentError.Detail + errorText := "Current error: " + state.currentError.status.Summary + if state.errorExpanded && state.currentError.detail != "" { + errorText += "\n" + state.currentError.detail } lines = append(lines, errorText) } @@ -165,7 +165,13 @@ func (p terminalPresentation) renderShell(page terminalPage, width, height int) status := "" if page.status != "" { wrapped := lipgloss.NewStyle().Width(width).Render(p.theme.status.Render("Status: " + page.status)) - status = viewportContent(wrapped, width, min(2, lineCount(wrapped)), 0) + statusHeight := min(2, lineCount(wrapped)) + statusOffset := 0 + if page.statusExpanded { + statusHeight = min(lineCount(wrapped), max(3, height/2)) + statusOffset = page.statusOffset + } + status = viewportContent(wrapped, width, statusHeight, statusOffset) } headerHeight := lineCount(header) diff --git a/internal/tui/progressive_detection_test.go b/internal/tui/progressive_detection_test.go index bfc7d6f..95b4d4d 100644 --- a/internal/tui/progressive_detection_test.go +++ b/internal/tui/progressive_detection_test.go @@ -14,52 +14,41 @@ import ( type progressiveDetectionModule struct { snapshot app.Snapshot - streams []chan app.TargetDetectionUpdate + streams []chan app.OperationUpdate begins int } func (m *progressiveDetectionModule) Snapshot() app.Snapshot { - return cloneDetectionSnapshot(m.snapshot) + snapshot := cloneDetectionSnapshot(m.snapshot) + if m.begins == 0 { + for i := range snapshot.Targets { + snapshot.Targets[i].DetectionPhase = app.TargetChecking + } + } + return snapshot } func (m *progressiveDetectionModule) Dispatch(app.Intent) app.Result { return app.Result{Snapshot: m.snapshot} } -func (m *progressiveDetectionModule) BeginTargetDetection() (app.Snapshot, <-chan app.TargetDetectionUpdate) { +func (m *progressiveDetectionModule) StartOperation(intent app.Intent) app.OperationStream { m.begins++ for i := range m.snapshot.Targets { m.snapshot.Targets[i].DetectionPhase = app.TargetChecking m.snapshot.Targets[i].DetectionErr = nil } - return cloneDetectionSnapshot(m.snapshot), m.streams[m.begins-1] -} -func (m *progressiveDetectionModule) StartOperation(intent app.Intent) app.OperationStream { - initial, detection := m.BeginTargetDetection() - updates := make(chan app.OperationUpdate, len(initial.Targets)+1) - go func() { - defer close(updates) - for update := range detection { - phase := app.OperationRunning - status := app.OperationStatus{Kind: app.StatusProgress, Summary: "Checking Targets"} - if update.Complete { - phase = app.OperationSucceeded - status = app.OperationStatus{Kind: app.StatusSuccess, Summary: "Target checks completed"} - } - updates <- app.OperationUpdate{Snapshot: update.Snapshot, Phase: phase, Status: status} - } - }() - return app.OperationStream{Snapshot: initial, Updates: updates} + return app.OperationStream{Snapshot: cloneDetectionSnapshot(m.snapshot), Updates: m.streams[m.begins-1]} } func TestCompleteModelStreamsStartupAndRefreshByTargetWithoutMovingSelection(t *testing.T) { - startup := make(chan app.TargetDetectionUpdate, 4) - refresh := make(chan app.TargetDetectionUpdate, 4) + startup := make(chan app.OperationUpdate, 4) + refresh := make(chan app.OperationUpdate, 4) module := &progressiveDetectionModule{ snapshot: app.Snapshot{Targets: []app.TargetSnapshot{ {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell description"}}, {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT description"}}, {ID: "docker", Description: target.Description{Name: "Docker", Summary: "Docker description"}}, }}, - streams: []chan app.TargetDetectionUpdate{startup, refresh}, + streams: []chan app.OperationUpdate{startup, refresh}, } model := resizedModel(t, module, themeModeNoColor, 120, 36) assertUsableFrame(t, model.View(), 120, 36, @@ -90,39 +79,40 @@ func TestCompleteModelStreamsStartupAndRefreshByTargetWithoutMovingSelection(t * failure := errors.New("apt-config inspection failed") startup <- module.detectionUpdate("shell", app.TargetDetectionFailed, false, "", "", failure) model, command = applyDetectionCommand(t, model, command) - assertUsableFrame(t, model.View(), 120, 36, "! Check failed Shell", "> … Checking APT", "● Enabled Docker") + assertUsableFrame(t, model.View(), 120, 36, "! Failed Shell", "> … Checking APT", "● Enabled Docker") startup <- module.detectionUpdate("apt", app.TargetReady, false, target.Disabled, "apt-config is unavailable", nil) model, command = applyDetectionCommand(t, model, command) - startup <- app.TargetDetectionUpdate{Snapshot: cloneDetectionSnapshot(module.snapshot), Complete: true} + startup <- app.FailedOperationUpdate(cloneDetectionSnapshot(module.snapshot), app.OperationStatus{Summary: "Some Target checks failed", TargetID: "shell"}, nil, "shell", failure) model, command = applyDetectionCommand(t, model, command) - if command != nil || model.selectedTargetID != "apt" { - t.Fatalf("startup completion command=%v selection=%q", command != nil, model.selectedTargetID) + if command != nil || model.selectedTargetID != "shell" { + t.Fatalf("failed startup did not focus the first failed Target: command=%v selection=%q", command != nil, model.selectedTargetID) } - assertUsableFrame(t, model.View(), 120, 36, "! Check failed Shell", "> ○ Unavailable APT", "● Enabled Docker", "Evidence: apt-config is unavailable") + assertUsableFrame(t, model.View(), 120, 36, "> ! Failed Shell", "● Enabled Docker") + model = updateKey(model, "down") updated, command := model.Update(keyMessage("r")) model = updated.(Model) if command == nil || module.begins != 2 { t.Fatalf("refresh did not reuse progressive detection: command=%v begins=%d", command != nil, module.begins) } - assertUsableFrame(t, model.View(), 120, 36, "… Checking Shell", "> … Checking APT", "… Checking Docker") + assertUsableFrame(t, model.View(), 120, 36, "… Checking Shell", "… Checking APT", "> … Checking Docker") refresh <- module.detectionUpdate("apt", app.TargetReady, true, target.Enabled, "apt enabled", nil) model, command = applyDetectionCommand(t, model, command) - assertUsableFrame(t, model.View(), 120, 36, "… Checking Shell", "> ● Enabled APT", "… Checking Docker") - if model.selectedTargetID != "apt" { + assertUsableFrame(t, model.View(), 120, 36, "… Checking Shell", "● Enabled APT", "> … Checking Docker") + if model.selectedTargetID != "docker" { t.Fatalf("refresh completion moved selection to %q", model.selectedTargetID) } } func TestCompleteModelReconcilesAuthoritativeSnapshotWhenDetectionGenerationClosesStale(t *testing.T) { - startup := make(chan app.TargetDetectionUpdate) + startup := make(chan app.OperationUpdate) module := &progressiveDetectionModule{ snapshot: app.Snapshot{Targets: []app.TargetSnapshot{{ ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell description"}, }}}, - streams: []chan app.TargetDetectionUpdate{startup}, + streams: []chan app.OperationUpdate{startup}, } model := resizedModel(t, module, themeModeNoColor, 80, 24) assertUsableFrame(t, model.View(), 80, 24, "> … Checking Shell") @@ -132,13 +122,13 @@ func TestCompleteModelReconcilesAuthoritativeSnapshotWhenDetectionGenerationClos _ = module.detectionUpdate("shell", app.TargetReady, true, target.Enabled, "shell enabled", nil) close(startup) model, next := applyDetectionCommand(t, model, model.Init()) - if next != nil || model.refreshing { - t.Fatalf("stale closure left detection active: next=%v refreshing=%v", next != nil, model.refreshing) + if next != nil || model.operation.active { + t.Fatalf("stale closure left detection active: next=%v active=%v", next != nil, model.operation.active) } assertUsableFrame(t, model.View(), 80, 24, "> ● Enabled Shell", "Evidence: shell enabled") } -func (m *progressiveDetectionModule) detectionUpdate(id string, phase app.TargetDetectionPhase, available bool, state target.State, evidence string, err error) app.TargetDetectionUpdate { +func (m *progressiveDetectionModule) detectionUpdate(id string, phase app.TargetDetectionPhase, available bool, state target.State, evidence string, err error) app.OperationUpdate { for i := range m.snapshot.Targets { if m.snapshot.Targets[i].ID != id { continue @@ -151,7 +141,11 @@ func (m *progressiveDetectionModule) detectionUpdate(id string, phase app.Target m.snapshot.Targets[i].Evidence = evidence break } - return app.TargetDetectionUpdate{Snapshot: cloneDetectionSnapshot(m.snapshot), TargetID: id} + progressPhase := app.TargetProgressSucceeded + if err != nil { + progressPhase = app.TargetProgressFailed + } + return app.RunningOperationUpdate(cloneDetectionSnapshot(m.snapshot), app.OperationStatus{Summary: "Checked Target", TargetID: id}, []app.TargetProgress{{TargetID: id, Phase: progressPhase, State: state, Evidence: evidence, Err: err}}) } func cloneDetectionSnapshot(snapshot app.Snapshot) app.Snapshot { diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index a6361bb..1dc8b4c 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -2,7 +2,6 @@ package tui import ( "errors" - "reflect" "strings" "testing" @@ -26,26 +25,20 @@ func (m *targetModule) Dispatch(intent app.Intent) app.Result { return m.result } func (m *targetModule) StartOperation(intent app.Intent) app.OperationStream { + initial := m.Snapshot() result := m.Dispatch(intent) updates := make(chan app.OperationUpdate, 1) - phase := app.OperationSucceeded - status := app.OperationStatus{Kind: app.StatusSuccess, Summary: result.Notice, TargetID: intent.TargetID} - if result.ConfirmationRequest != nil || result.Confirmation != "" { - phase = app.OperationAwaitingConfirmation - confirmation := result.ConfirmationRequest - if confirmation == nil { - confirmation = &app.Confirmation{Title: "Confirm Target change", Summary: result.Confirmation, ConfirmLabel: "Continue", CancelLabel: "Cancel"} - } - updates <- app.OperationUpdate{Snapshot: result.Snapshot, Phase: phase, Status: app.OperationStatus{Kind: app.StatusConfirmation, Summary: confirmation.Title}, Confirmation: confirmation} - } else { - if result.Err != nil { - phase = app.OperationFailed - status = app.OperationStatus{Kind: app.StatusError, Summary: "Operation failed", Detail: result.Err.Error(), TargetID: intent.TargetID} - } - updates <- app.OperationUpdate{Snapshot: result.Snapshot, Phase: phase, Status: status, Err: result.Err} + status := app.OperationStatus{Summary: result.Notice, TargetID: intent.TargetID} + switch { + case result.ConfirmationRequest != nil: + updates <- app.ConfirmationOperationUpdate(result.Snapshot, *result.ConfirmationRequest) + case result.Err != nil: + updates <- app.FailedOperationUpdate(result.Snapshot, app.OperationStatus{Summary: "Operation failed", TargetID: intent.TargetID}, nil, intent.TargetID, result.Err) + default: + updates <- app.SucceededOperationUpdate(result.Snapshot, status, nil) } close(updates) - return app.OperationStream{Snapshot: m.snapshot, Updates: updates} + return app.OperationStream{Snapshot: initial, Updates: updates} } func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing.T) { @@ -75,7 +68,7 @@ func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing func TestTargetRestartConfirmationCancelsWithoutConfirmedMutation(t *testing.T) { initial := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "docker-daemon", Description: target.Description{Name: "Docker daemon"}, Available: true, State: target.Disabled}}} - module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, Confirmation: "Docker will restart immediately and may disrupt running work."}} + 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}) model = updated.(Model) @@ -155,7 +148,7 @@ func TestBusyActivationBlocksPageEntryAndEveryNewOperation(t *testing.T) { Profiles: []profile.Profile{{Name: "new"}}, Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}, } - module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: make(chan app.Result)} + module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: make(chan app.OperationUpdate)} model := updateKey(NewModel(module), "p") updated, command := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) model = updated.(Model) @@ -172,8 +165,8 @@ func TestBusyActivationBlocksPageEntryAndEveryNewOperation(t *testing.T) { } updated, quit := model.Update(keyMessage("q")) model = updated.(Model) - if quit != nil || !model.quitRequested || len(module.intents) != 1 { - t.Fatalf("busy activation did not defer quit: command=%v requested=%v intents=%#v", quit != nil, model.quitRequested, module.intents) + if quit != nil || !model.operation.quitRequested || len(module.intents) != 1 { + t.Fatalf("busy activation did not defer quit: command=%v requested=%v intents=%#v", quit != nil, model.operation.quitRequested, module.intents) } // Even if a Profiles-page activation key is delivered while the transition @@ -208,154 +201,11 @@ func TestBusyRefreshBlocksTogglePageEntryRefreshAndActivation(t *testing.T) { } updated, quit := model.Update(keyMessage("q")) model = updated.(Model) - if quit != nil || !model.quitRequested || len(module.intents) != 1 { + if quit != nil || !model.operation.quitRequested || len(module.intents) != 1 { t.Fatalf("busy refresh did not defer quit: command=%v intents=%#v", quit != nil, module.intents) } } -func TestPendingActivationRemainsBusyUntilConfirmationIsResolved(t *testing.T) { - initial := app.Snapshot{Profiles: []profile.Profile{{Name: "office"}}} - updates := make(chan app.Result, 1) - updates <- app.Result{Snapshot: initial, Confirmation: "confirm transition"} - module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: updates} - model := updateKey(NewModel(module), "p") - updated, command := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) - model = updated.(Model) - updated, _ = model.Update(command()) - model = updated.(Model) - if !model.busy() || model.pendingActivation.Kind != app.ActivateProfile || model.profileConfirm == nil { - t.Fatalf("pending activation was not guarded: %#v", model) - } - - model.page = mainPage - for _, key := range []string{"p", "?", "r", " "} { - updated, blocked := model.Update(keyMessage(key)) - model = updated.(Model) - if blocked != nil || model.page != mainPage || len(module.intents) != 1 { - t.Fatalf("pending activation accepted %q: page=%v command=%v intents=%#v", key, model.page, blocked != nil, module.intents) - } - } - model.page = profilesPage - updated, _ = model.Update(keyMessage("n")) - model = updated.(Model) - if model.busy() || model.profileConfirm != nil { - t.Fatal("cancelling confirmation did not clear pending activation") - } -} - -func TestProfileConfirmationQuitKeysCancelWithoutLeavingBusyState(t *testing.T) { - for _, cancelKey := range []tea.KeyMsg{keyMessage("q"), {Type: tea.KeyEsc}} { - t.Run(cancelKey.String(), func(t *testing.T) { - initial := app.Snapshot{Profiles: []profile.Profile{{Name: "office"}}} - updates := make(chan app.Result, 2) - updates <- app.Result{Snapshot: initial, Confirmation: "confirm transition"} - module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: updates} - model := updateKey(NewModel(module), "p") - updated, command := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) - model = updated.(Model) - updated, command = model.Update(command()) - model = updated.(Model) - if command != nil || model.profileConfirm == nil || !model.busy() { - t.Fatal("activation confirmation was not presented") - } - - updated, command = model.Update(cancelKey) - model = updated.(Model) - if command != nil || model.profileConfirm != nil || model.busy() || model.quitRequested { - t.Fatalf("confirmation cancel %q left the model blocked: %#v", cancelKey.String(), model) - } - - updates <- app.Result{Snapshot: initial, Confirmation: "confirm transition again"} - model = updateKey(model, "p") - updated, command = model.Update(tea.KeyMsg{Type: tea.KeyEnter}) - if command == nil { - t.Fatal("confirmation cancellation prevented a later activation") - } - }) - } -} - -func TestDeferredQuitIsTerminalAfterTargetConfirmationCompletion(t *testing.T) { - initial := app.Snapshot{ - ActiveProfile: "office", - Profiles: []profile.Profile{{Name: "office"}}, - Targets: []app.TargetSnapshot{{ID: "docker-daemon", Description: target.Description{Name: "Docker daemon"}, Available: true, State: target.Disabled}}, - } - module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial, Confirmation: "Docker will restart immediately."}} - model := NewModel(module) - updated, operation := model.Update(tea.KeyMsg{Type: tea.KeySpace}) - model = updated.(Model) - updated, command := model.Update(keyMessage("q")) - model = updated.(Model) - if command != nil || !model.quitRequested { - t.Fatal("quit was not deferred during Target operation") - } - updated, queuedQuit := model.Update(operation()) - model = updated.(Model) - if queuedQuit == nil || model.targetConfirm == nil || len(module.intents) != 1 { - t.Fatalf("confirmation completion did not queue quit: command=%v confirmation=%v intents=%#v", queuedQuit != nil, model.targetConfirm != nil, module.intents) - } - - baselineSnapshot := model.snapshot - baselineStatus := model.status - baselinePage := model.page - baselineSelected := model.selected - baselineConfirmation := *model.targetConfirm - for _, message := range []tea.Msg{ - keyMessage("y"), keyMessage("n"), tea.KeyMsg{Type: tea.KeyEsc}, keyMessage(" "), - keyMessage("r"), keyMessage("p"), keyMessage("?"), keyMessage("j"), queuedQuit(), - } { - updated, command = model.Update(message) - model = updated.(Model) - if command != nil { - t.Fatalf("terminal deferred-quit model returned a command for %T", message) - } - if !model.quitRequested || !reflect.DeepEqual(model.snapshot, baselineSnapshot) || model.status != baselineStatus || model.page != baselinePage || model.selected != baselineSelected || model.targetConfirm == nil || !reflect.DeepEqual(*model.targetConfirm, baselineConfirmation) || len(module.intents) != 1 { - t.Fatalf("terminal deferred-quit model changed for %T: model=%#v intents=%#v", message, model, module.intents) - } - } -} - -func TestDeferredQuitIsTerminalAfterProfileConfirmationCompletion(t *testing.T) { - initial := app.Snapshot{Profiles: []profile.Profile{{Name: "office"}}} - updates := make(chan app.Result, 1) - updates <- app.Result{Snapshot: initial, Confirmation: "confirm transition"} - module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: updates} - model := updateKey(NewModel(module), "p") - updated, operation := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) - model = updated.(Model) - updated, command := model.Update(keyMessage("q")) - model = updated.(Model) - if command != nil || !model.quitRequested { - t.Fatal("quit was not deferred during Profile activation") - } - updated, queuedQuit := model.Update(operation()) - model = updated.(Model) - if queuedQuit == nil || model.profileConfirm == nil || len(module.intents) != 1 { - t.Fatalf("Profile confirmation completion did not queue quit: command=%v confirmation=%v intents=%#v", queuedQuit != nil, model.profileConfirm != nil, module.intents) - } - - baselineSnapshot := model.snapshot - baselineStatus := model.status - baselinePage := model.page - baselineSelected := model.selected - baselinePending := model.pendingActivation - baselineConfirmation := *model.profileConfirm - for _, message := range []tea.Msg{ - keyMessage("y"), keyMessage("n"), tea.KeyMsg{Type: tea.KeyEsc}, tea.KeyMsg{Type: tea.KeyEnter}, - keyMessage("d"), keyMessage("e"), keyMessage("a"), keyMessage("r"), queuedQuit(), - } { - updated, command = model.Update(message) - model = updated.(Model) - if command != nil { - t.Fatalf("terminal deferred-quit Profile model returned a command for %T", message) - } - if !model.quitRequested || !reflect.DeepEqual(model.snapshot, baselineSnapshot) || model.status != baselineStatus || model.page != baselinePage || model.selected != baselineSelected || !reflect.DeepEqual(model.pendingActivation, baselinePending) || model.profileConfirm == nil || !reflect.DeepEqual(*model.profileConfirm, baselineConfirmation) || len(module.intents) != 1 { - t.Fatalf("terminal deferred-quit Profile model changed for %T: model=%#v intents=%#v", message, model, module.intents) - } - } -} - func TestRefreshClampsSelectionAfterMultipleTargetsDisappear(t *testing.T) { initial := app.Snapshot{Targets: []app.TargetSnapshot{ {ID: "one", Description: target.Description{Name: "One"}, Available: true}, @@ -411,14 +261,16 @@ func TestTargetPanelNavigationKeysAndQuitCombinations(t *testing.T) { } } } - 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()) - } + _, command := NewModel(module).Update(keyMessage("q")) + if command == nil { + t.Fatal("dashboard q returned no command") + } + if _, ok := command().(tea.QuitMsg); !ok { + t.Fatal("dashboard q did not return tea.QuitMsg") + } + _, command = NewModel(module).Update(tea.KeyMsg{Type: tea.KeyEsc}) + if command != nil { + t.Fatal("dashboard Escape must remain cancel/back rather than exit") } } @@ -440,59 +292,21 @@ func TestQAndEscapeReturnFromNestedPages(t *testing.T) { type progressiveTargetModule struct { targetModule - updates chan app.Result + updates chan app.OperationUpdate } -func (m *progressiveTargetModule) DispatchProgress(intent app.Intent) <-chan app.Result { - m.intents = append(m.intents, intent) - return m.updates -} func (m *progressiveTargetModule) StartOperation(intent app.Intent) app.OperationStream { m.intents = append(m.intents, intent) - updates := make(chan app.OperationUpdate, cap(m.updates)+1) - go func() { - defer close(updates) - for result := range m.updates { - phase := app.OperationRunning - status := app.OperationStatus{Kind: app.StatusProgress, Summary: result.Notice} - var confirmation *app.Confirmation - if result.Confirmation != "" { - phase = app.OperationAwaitingConfirmation - confirmation = &app.Confirmation{Title: "Confirm Active Profile change", Summary: result.Confirmation, ConfirmLabel: "Continue", CancelLabel: "Cancel"} - status = app.OperationStatus{Kind: app.StatusConfirmation, Summary: confirmation.Title} - } else if result.Complete { - phase = app.OperationSucceeded - status.Kind = app.StatusSuccess - if result.Err != nil { - phase = app.OperationFailed - status = app.OperationStatus{Kind: app.StatusError, Summary: "Active Profile change failed", Detail: result.Err.Error()} - } - } - progress := append([]app.TargetProgress(nil), result.Progress...) - for i := range progress { - if progress[i].Applying { - progress[i].Phase = app.TargetProgressRunning - } else if progress[i].Err != nil { - progress[i].Phase = app.TargetProgressFailed - } else { - progress[i].Phase = app.TargetProgressSucceeded - } - } - updates <- app.OperationUpdate{Snapshot: result.Snapshot, Phase: phase, Status: status, Progress: progress, Confirmation: confirmation, Err: result.Err} - if phase == app.OperationAwaitingConfirmation { - return - } - } - }() - return app.OperationStream{Snapshot: m.snapshot, Updates: updates} + return app.OperationStream{Snapshot: m.snapshot, Updates: m.updates} } func TestActiveProfileTransitionRendersPerTargetProgress(t *testing.T) { initial := app.Snapshot{ActiveProfile: "old", Profiles: []profile.Profile{{Name: "new"}}, Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}} - updates := make(chan app.Result, 3) - updates <- app.Result{Snapshot: app.Snapshot{ActiveProfile: "new", Targets: initial.Targets}, Progress: []app.TargetProgress{{TargetID: "shell", Applying: true}}} - updates <- app.Result{Snapshot: app.Snapshot{ActiveProfile: "new", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}}, Progress: []app.TargetProgress{{TargetID: "shell", State: target.Enabled}}, Notice: "Shell Target update completed"} - updates <- app.Result{Snapshot: app.Snapshot{ActiveProfile: "new", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}}, Complete: true, Notice: "Activated Profile new"} + updates := make(chan app.OperationUpdate, 3) + updates <- app.RunningOperationUpdate(app.Snapshot{ActiveProfile: "new", Targets: initial.Targets}, app.OperationStatus{Summary: "Updating Shell Target", TargetID: "shell"}, []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressRunning}}) + completedSnapshot := app.Snapshot{ActiveProfile: "new", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}} + updates <- app.RunningOperationUpdate(completedSnapshot, app.OperationStatus{Summary: "Shell Target update completed", TargetID: "shell"}, []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressSucceeded, State: target.Enabled}}) + updates <- app.SucceededOperationUpdate(completedSnapshot, app.OperationStatus{Summary: "Activated Profile new"}, []app.TargetProgress{{TargetID: "shell", Phase: app.TargetProgressSucceeded, State: target.Enabled}}) close(updates) module := &progressiveTargetModule{targetModule: targetModule{snapshot: initial}, updates: updates} model := updateKey(NewModel(module), "p")