From 177bf343541d03e52746450d60b4396a5c084208 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 21:46:06 +0800 Subject: [PATCH] pi-agent: Implement operation coordination --- internal/app/app.go | 107 +++++-- internal/app/operation.go | 228 +++++++++++++++ internal/app/operation_stream_test.go | 149 ++++++++++ internal/tui/foundation_test.go | 4 +- internal/tui/model.go | 304 +++++++++++++++----- internal/tui/model_test.go | 21 +- internal/tui/operation_coordination_test.go | 184 ++++++++++++ internal/tui/pages.go | 44 ++- internal/tui/presentation.go | 28 +- internal/tui/progressive_detection_test.go | 17 ++ internal/tui/target_test.go | 171 +++++------ 11 files changed, 1056 insertions(+), 201 deletions(-) create mode 100644 internal/app/operation.go create mode 100644 internal/app/operation_stream_test.go create mode 100644 internal/tui/operation_coordination_test.go diff --git a/internal/app/app.go b/internal/app/app.go index 73f0bf2..26483ac 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -70,19 +70,26 @@ 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 Err error } type Result struct { - Snapshot Snapshot - Notice string - Confirmation string - Progress []TargetProgress - Complete bool - Err error + Snapshot Snapshot + Notice string + // Confirmation remains for existing synchronous callers. + Confirmation string + ConfirmationRequest *Confirmation + Status OperationStatus + Progress []TargetProgress + FirstFailedTargetID string + Complete bool + Err error } type Module interface { @@ -105,20 +112,22 @@ type ProgressiveDetectionModule interface { } type Application struct { - store profile.Store - state profile.State - targets []target.Target - targetIDs []string - targetDescriptions []target.Description - availabilities map[string]target.Availability - inspections map[string]target.Inspection - detectionPhase map[string]TargetDetectionPhase - detectionErr map[string]error - detectionGeneration uint64 - stateMu sync.RWMutex - dispatchMu sync.Mutex - mutationLock MutationLock - logger OperationLogger + store profile.Store + state profile.State + targets []target.Target + targetIDs []string + targetDescriptions []target.Description + availabilities map[string]target.Availability + inspections map[string]target.Inspection + detectionPhase map[string]TargetDetectionPhase + detectionErr map[string]error + detectionGeneration uint64 + stateMu sync.RWMutex + dispatchMu sync.Mutex + interactiveMu sync.Mutex + interactiveOperation bool + mutationLock MutationLock + logger OperationLogger } func New(store profile.Store) (*Application, error) { @@ -290,7 +299,7 @@ func (a *Application) dispatch(intent Intent, publish func(Result)) Result { a.refreshInspections() } if intent.Kind == SetTargetDesired { - return a.setTargetDesired(intent) + return a.setTargetDesired(intent, publish) } if intent.Kind == RefreshTargets { a.refreshTargets() @@ -357,7 +366,13 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul } } if len(warnings) > 0 && !intent.Confirm { - return Result{Snapshot: a.Snapshot(), Confirmation: "Changing the Active Profile requires confirmation:\n" + strings.Join(warnings, "\n")} + message := "Changing the Active Profile requires confirmation:\n" + strings.Join(warnings, "\n") + confirmation := &Confirmation{ + 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}} } next, err := a.store.Mutate(func(current profile.State) (profile.State, error) { @@ -395,7 +410,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, State: a.inspection(id).State}}, Notice: "Updating " + name + " Target"}) + 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}}) } operation := adapter.SetDesired(context.Background(), true, active) inspection := operation.Inspection @@ -407,10 +423,18 @@ func (a *Application) activateProfile(intent Intent, publish func(Result)) Resul safeErr = fmt.Errorf("%s (fresh inspection: %s — %s)", profile.RedactText(operation.Err.Error()), inspection.State, profile.RedactText(inspection.Evidence)) } a.replaceInspection(id, inspection) - item := TargetProgress{TargetID: id, State: inspection.State, Evidence: inspection.Evidence, Err: safeErr} + progressPhase := TargetProgressSucceeded + if safeErr != nil { + progressPhase = TargetProgressFailed + } + item := TargetProgress{TargetID: id, Phase: progressPhase, State: inspection.State, Evidence: inspection.Evidence, Err: safeErr} progress = append(progress, item) if publish != nil { - publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{item}, Notice: name + " Target update completed", Err: safeErr}) + status := OperationStatus{Kind: StatusSuccess, Summary: name + " Target update completed", TargetID: id} + if safeErr != nil { + status = OperationStatus{Kind: StatusError, Summary: name + " Target update failed", Detail: safeErr.Error(), TargetID: id} + } + publish(Result{Snapshot: a.Snapshot(), Progress: []TargetProgress{item}, Notice: name + " Target update completed", Status: status, Err: safeErr}) } if a.logger != nil { outcome, detail := "success", inspection.Evidence @@ -433,10 +457,19 @@ 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)} + 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)}} 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()} + for _, item := range progress { + if item.Err != nil { + result.FirstFailedTargetID = item.TargetID + result.Status.TargetID = item.TargetID + break + } + } } return result } @@ -530,7 +563,7 @@ func (a *Application) refreshInspections() { a.replaceInspections(inspections) } -func (a *Application) setTargetDesired(intent Intent) Result { +func (a *Application) setTargetDesired(intent Intent, publish func(Result)) Result { selectedIndex := -1 for i, id := range a.targetIDs { if id == intent.TargetID { @@ -547,7 +580,14 @@ func (a *Application) setTargetDesired(intent Intent) Result { return Result{Snapshot: a.Snapshot(), Err: errors.New("no Active Profile; open Profile management before enabling a Target")} } if warning := selected.Confirmation(intent.Enabled); warning != "" && !intent.Confirm { - return Result{Snapshot: a.Snapshot(), Confirmation: warning} + 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}} + } + 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}}) } result := selected.SetDesired(context.Background(), intent.Enabled, active) inspection := result.Inspection @@ -567,10 +607,19 @@ func (a *Application) setTargetDesired(intent Intent) Result { _ = a.logger.Write(LogEntry{Time: time.Now(), Operation: operation, Target: intent.TargetID, Result: outcome, Detail: RedactLogDetail(detail)}) } notice := "" + progressPhase := TargetProgressSucceeded + status := OperationStatus{Kind: StatusSuccess, 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() } - return Result{Snapshot: a.Snapshot(), Notice: notice, Err: result.Err} + 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} } func create(state profile.State, input profile.Profile) (profile.State, string, error) { diff --git a/internal/app/operation.go b/internal/app/operation.go new file mode 100644 index 0000000..969b91d --- /dev/null +++ b/internal/app/operation.go @@ -0,0 +1,228 @@ +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 + +const ( + StatusProgress StatusKind = iota + 1 + 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. +type OperationStatus struct { + Kind StatusKind + Summary string + Detail string + TargetID string +} + +// Confirmation is a structured, safe-default decision. Presentation layers +// render these fields but never infer confirmation state from their wording. +type Confirmation struct { + Title string + Summary string + Details []string + ConfirmLabel string + CancelLabel string + HighImpact bool +} + +// TargetProgressPhase describes one Target's part in an operation stream. +type TargetProgressPhase uint8 + +const ( + TargetProgressRunning TargetProgressPhase = iota + 1 + TargetProgressSucceeded + 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 +} + +// OperationStream exposes the useful initial frame synchronously and all +// subsequent structured changes through Updates. Updates closes after a +// terminal phase. +type OperationStream struct { + Snapshot Snapshot + Updates <-chan OperationUpdate +} + +// InteractiveModule is the presentation-independent Application seam used by +// the TUI. Module.Dispatch remains available for synchronous callers. +type InteractiveModule interface { + Module + StartOperation(Intent) OperationStream +} + +// StartOperation rejects overlap instead of queuing hidden interactive work. +// Synchronous Dispatch callers retain the established serialized behavior. +func (a *Application) StartOperation(intent Intent) OperationStream { + 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."}, + } + close(updates) + return OperationStream{Snapshot: a.Snapshot(), 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 <- 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} + } + }() + return OperationStream{Snapshot: initial, Updates: updates} +} + +func (a *Application) finishInteractiveOperation() { + a.interactiveMu.Lock() + a.interactiveOperation = false + 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 + } + } + return TargetSnapshot{ID: id}, id +} + +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, + } + 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 + } + 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 + } + } + return update +} diff --git a/internal/app/operation_stream_test.go b/internal/app/operation_stream_test.go new file mode 100644 index 0000000..0d31e3c --- /dev/null +++ b/internal/app/operation_stream_test.go @@ -0,0 +1,149 @@ +package app_test + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" + + "proxyctl/internal/app" + "proxyctl/internal/profile" + "proxyctl/internal/target" +) + +type coordinatedOperationTarget struct { + id string + state target.State + started chan struct{} + release chan struct{} + confirmation string + failure error +} + +func (t *coordinatedOperationTarget) ID() string { return t.id } +func (t *coordinatedOperationTarget) Description() target.Description { + return target.Description{Name: t.id, Summary: "operation stream test Target"} +} +func (t *coordinatedOperationTarget) Availability(context.Context) target.Availability { + return target.Availability{Available: true, Evidence: "available"} +} +func (t *coordinatedOperationTarget) Inspect(context.Context, *profile.Profile) target.Inspection { + return target.Inspection{State: t.state, Evidence: "fresh state"} +} +func (t *coordinatedOperationTarget) Confirmation(bool) string { return t.confirmation } +func (t *coordinatedOperationTarget) SetDesired(_ context.Context, enabled bool, _ *profile.Profile) target.OperationResult { + if t.started != nil { + close(t.started) + } + if t.release != nil { + <-t.release + } + if t.failure != nil { + t.state = target.Drifted + return target.OperationResult{Err: t.failure} + } + if enabled { + t.state = target.Enabled + } else { + t.state = target.Disabled + } + return target.OperationResult{Inspection: target.Inspection{State: t.state, Evidence: "completed"}} +} + +func TestInteractiveOperationStreamSerializesMutationAndPublishesStructuredProgress(t *testing.T) { + adapter := &coordinatedOperationTarget{id: "shell", state: target.Disabled, 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) + } + 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) + } + + 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 { + 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") { + t.Fatalf("blocked update = %#v", blockedUpdate) + } + if _, open := <-blocked.Updates; open { + t.Fatal("blocked operation stream did not close") + } + + 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 { + t.Fatalf("final update = %#v", final) + } + if _, open := <-operation.Updates; open { + t.Fatal("completed operation stream did not close") + } +} + +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}) + 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) + } + + 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 { + t.Fatalf("confirmation update = %#v", update) + } + if _, open := <-operation.Updates; open { + t.Fatal("confirmation operation stream did not close") + } +} + +func TestInteractiveProfileTransitionReportsFirstFailureWithoutRollback(t *testing.T) { + first := &coordinatedOperationTarget{id: "first", state: target.Disabled} + second := &coordinatedOperationTarget{id: "second", state: target.Disabled} + application, err := app.NewWithTargets(profile.NewFileStore(filepath.Join(t.TempDir(), "profiles.json")), []target.Target{first, second}) + if err != nil { + t.Fatal(err) + } + for _, name := range []string{"old", "new"} { + if result := application.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{Name: name, SingleURL: "http://proxy.example:8080"}}); result.Err != nil { + t.Fatal(result.Err) + } + } + if result := application.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil { + t.Fatal(result.Err) + } + for _, id := range []string{"first", "second"} { + if result := application.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: id, Enabled: true}); result.Err != nil { + t.Fatal(result.Err) + } + } + second.failure = errors.New("second failed") + + operation := application.StartOperation(app.Intent{Kind: app.ActivateProfile, Name: "new"}) + var final app.OperationUpdate + for update := range operation.Updates { + final = update + } + if final.Phase != app.OperationFailed || 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 { + t.Fatalf("partial Target states = %#v", final.Snapshot.Targets) + } +} diff --git a/internal/tui/foundation_test.go b/internal/tui/foundation_test.go index ec3f29d..dbb4b16 100644 --- a/internal/tui/foundation_test.go +++ b/internal/tui/foundation_test.go @@ -172,7 +172,9 @@ func TestCompleteModelRedactsApplicationErrorsBeforeRendering(t *testing.T) { updated, cmd := model.Update(keyMessage("r")) model = updated.(Model) updated, _ = model.Update(cmd()) - view := updated.(Model).View() + model = updated.(Model) + model = updateKey(model, "e") + view := model.View() if strings.Contains(view, secret) || !strings.Contains(view, "https://bob:REDACTED@proxy.example:8080") { t.Fatalf("Application error bypassed presentation redaction:\n%s", view) } diff --git a/internal/tui/model.go b/internal/tui/model.go index 630a32a..5efb828 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -48,6 +48,15 @@ type Model struct { 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 @@ -76,6 +85,12 @@ type profileOperationProgress struct { updates <-chan app.Result } +type operationProgress struct { + update app.OperationUpdate + updates <-chan app.OperationUpdate + closed bool +} + // probeDone carries one finished Protocol Probe back to the form. address is // always bare host:port so status text can never leak credentials. gen matches // the probe generation stored in formState so late results can be ignored after @@ -97,6 +112,16 @@ func waitForTargetDetection(updates <-chan app.TargetDetectionUpdate) tea.Cmd { } } +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 @@ -113,26 +138,35 @@ func NewModel(application app.Module, configure ...ModelOption) Model { option(&options) } snapshot := application.Snapshot() - var detectionUpdates <-chan app.TargetDetectionUpdate - if progressive, ok := application.(app.ProgressiveDetectionModule); ok { - snapshot, detectionUpdates = progressive.BeginTargetDetection() + 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, } - return Model{ - application: application, - presentation: terminalPresentation{theme: newSemanticTheme(options.theme, options.output)}, - snapshot: snapshot, - selectedTargetID: newTargetCatalog(snapshot, false, "").firstID(), - width: 80, - height: 24, - refreshing: detectionUpdates != nil, - detectionUpdates: detectionUpdates, - detect: probe.Detect, + _, progressiveDetection := application.(app.ProgressiveDetectionModule) + if targetsChecking(snapshot) || progressiveDetection { + model.startOperation(app.Intent{Kind: app.RefreshTargets}) } + return model +} + +func targetsChecking(snapshot app.Snapshot) bool { + for _, item := range snapshot.Targets { + if item.DetectionPhase == app.TargetChecking { + return true + } + } + return false } func (m Model) Init() tea.Cmd { - if m.detectionUpdates != nil { - return waitForTargetDetection(m.detectionUpdates) + if m.operationUpdates != nil { + return waitForOperation(m.operationUpdates) } return nil } @@ -145,11 +179,11 @@ func (m Model) busy() bool { } func (m Model) operationInFlight() bool { - return m.applying || m.refreshing || m.profileTransition + return m.operationActive || m.applying || m.refreshing || m.profileTransition } func (m Model) dashboardNavigationAvailable() bool { - return !m.quitRequested && !m.applying && !m.profileTransition + return !m.quitRequested } func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { @@ -166,9 +200,32 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { if m.quitRequested && !m.operationInFlight() { return m, nil } - if key, ok := message.(tea.KeyMsg); ok && m.operationInFlight() && isQuitKey(key) { - m.quitRequested = true - 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) + if exitRequest { + if m.operationInFlight() { + m.quitRequested = true + return m, nil + } + return m, tea.Quit + } + } + if progress, ok := message.(operationProgress); ok { + if !m.operationActive || progress.updates != m.operationUpdates { + return m, nil + } + if progress.closed { + m.snapshot = m.application.Snapshot() + m.retainTargetSelection() + m.finishOperation() + return m, m.deferredQuit() + } + m.applyOperationUpdate(progress.update) + if isTerminalOperationPhase(progress.update.Phase) { + m.finishOperation() + return m, m.deferredQuit() + } + return m, waitForOperation(progress.updates) } if progress, ok := message.(targetDetectionProgress); ok { if !m.refreshing || progress.updates != m.detectionUpdates { @@ -338,6 +395,106 @@ 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 + } + 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) +} + +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 (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": @@ -355,46 +512,58 @@ func (m Model) deferredQuit() tea.Cmd { } func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { - if m.targetConfirm != nil && !m.busy() { + if m.targetConfirm != nil || m.profileConfirm != nil { + intent := m.targetConfirm + if intent == nil { + intent = m.profileConfirm + } switch key.String() { case "y": - intent := *m.targetConfirm - intent.Confirm = true + confirmed := *intent + confirmed.Confirm = true m.targetConfirm = nil - m.applying = true + m.profileConfirm = nil + m.pendingActivation = app.Intent{} + m.confirmation = nil m.status = "" - return m, func() tea.Msg { return targetOperationDone{result: m.application.Dispatch(intent)} } - case "n", "esc", "q": + return m, m.startOperation(confirmed) + case "enter", "n", "esc", "q": m.targetConfirm = nil - m.status = "" + m.profileConfirm = nil + m.pendingActivation = app.Intent{} + m.confirmation = nil + m.operationStatus = app.OperationStatus{} } return m, nil } + if key.String() == "e" && m.currentError != nil { + m.errorExpanded = !m.errorExpanded + return m, nil + } + if key.String() == "x" && m.currentError != nil { + m.currentError = nil + m.errorExpanded = false + return m, nil + } catalog := newTargetCatalog(m.snapshot, m.showUnavailable, m.selectedTargetID) switch key.String() { case "p": - if !m.busy() { + if m.operationActive || m.quitRequested { + m.operationStatus = 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 = "" } case "?": - if !m.busy() { + if !m.quitRequested { m.page = helpPage - m.status = "" } case "r": - if !m.busy() { - m.refreshing = true - m.status = "Refreshing Target availability and state…" - if progressive, ok := m.application.(app.ProgressiveDetectionModule); ok { - m.snapshot, m.detectionUpdates = progressive.BeginTargetDetection() - m.retainTargetSelection() - return m, waitForTargetDetection(m.detectionUpdates) - } - return m, func() tea.Msg { - return refreshDone{result: m.application.Dispatch(app.Intent{Kind: app.RefreshTargets})} - } + if m.operationActive || m.quitRequested { + m.operationStatus = 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}) } case "tab": if m.dashboardNavigationAvailable() { @@ -438,22 +607,21 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { m.detailsOffset += 5 } case " ": - if !m.busy() { - selected, ok := catalog.selected(m.selectedTargetID) - if !ok || !selected.Available || selected.DetectionPhase != app.TargetReady { - break - } - desired := selected.State == target.Disabled || selected.State == target.External - m.applying = true - m.status = "" - intent := app.Intent{Kind: app.SetTargetDesired, TargetID: selected.ID, Enabled: desired} - m.targetConfirm = &intent - return m, func() tea.Msg { - return targetOperationDone{result: m.application.Dispatch(intent)} - } + 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 { + 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 } - case "q", "esc", "ctrl+c": - return m, tea.Quit } return m, nil } @@ -610,20 +778,13 @@ 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"} return m, nil } - if progressive, ok := m.application.(app.ProgressiveModule); ok { - m.applying = true - m.profileTransition = true - m.pendingActivation = intent - m.page = mainPage - m.selected = 0 - m.status = "Changing Active Profile…" - updates := progressive.DispatchProgress(intent) - return m, waitForProfileProgress(updates) - } - m.apply(intent) - return m, nil + m.pendingActivation = intent + m.page = mainPage + m.selected = 0 + return m, m.startOperation(intent) } func (m *Model) apply(intent app.Intent) { @@ -683,7 +844,14 @@ func (m Model) View() string { confirm: m.confirm, targetConfirm: m.targetConfirm != nil, profileConfirm: m.profileConfirm != nil, + confirmation: m.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, diff --git a/internal/tui/model_test.go b/internal/tui/model_test.go index c3b7d6b..9980735 100644 --- a/internal/tui/model_test.go +++ b/internal/tui/model_test.go @@ -23,6 +23,19 @@ func (m *recordingModule) Dispatch(intent app.Intent) app.Result { m.intents = append(m.intents, intent) return app.Result{Snapshot: m.snapshot, Notice: "done"} } +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} + if result.Err != nil { + phase = app.OperationFailed + status = app.OperationStatus{Kind: app.StatusError, Summary: "Operation failed", Detail: result.Err.Error()} + } + updates <- app.OperationUpdate{Snapshot: result.Snapshot, Phase: phase, Status: status, Err: result.Err} + close(updates) + return app.OperationStream{Snapshot: m.snapshot, Updates: updates} +} func TestMainAndProfilePagesMapKeysAndObscurePasswords(t *testing.T) { module := &recordingModule{snapshot: app.Snapshot{ @@ -46,10 +59,14 @@ func TestMainAndProfilePagesMapKeysAndObscurePasswords(t *testing.T) { t.Fatalf("Profile page exposed proxy password:\n%s", view) } - model = updateKey(model, "a") - if len(module.intents) != 1 || module.intents[0].Kind != app.ActivateProfile || module.intents[0].Name != "office" { + updated, command := model.Update(keyMessage("a")) + model = updated.(Model) + if command == nil || len(module.intents) != 1 || module.intents[0].Kind != app.ActivateProfile || module.intents[0].Name != "office" { t.Fatalf("activate key dispatched %#v", module.intents) } + updated, _ = model.Update(command()) + model = updated.(Model) + model = updateKey(model, "p") model = updateKey(model, "d") if !strings.Contains(model.View(), "Delete Profile") { t.Fatalf("delete key did not request confirmation:\n%s", model.View()) diff --git a/internal/tui/operation_coordination_test.go b/internal/tui/operation_coordination_test.go new file mode 100644 index 0000000..8ee41b7 --- /dev/null +++ b/internal/tui/operation_coordination_test.go @@ -0,0 +1,184 @@ +package tui + +import ( + "errors" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "proxyctl/internal/app" + "proxyctl/internal/profile" + "proxyctl/internal/target" +) + +type interactiveScriptModule struct { + snapshot app.Snapshot + streams []chan app.OperationUpdate + intents []app.Intent +} + +func (m *interactiveScriptModule) Snapshot() app.Snapshot { return m.snapshot } +func (m *interactiveScriptModule) Dispatch(intent app.Intent) app.Result { + m.intents = append(m.intents, intent) + return app.Result{Snapshot: m.snapshot} +} +func (m *interactiveScriptModule) StartOperation(intent app.Intent) app.OperationStream { + m.intents = append(m.intents, intent) + stream := m.streams[0] + m.streams = m.streams[1:] + return app.OperationStream{Snapshot: m.snapshot, Updates: stream} +} + +func operationSnapshot() app.Snapshot { + return app.Snapshot{ + ActiveProfile: "office", + Profiles: []profile.Profile{{Name: "office"}, {Name: "remote"}}, + Targets: []app.TargetSnapshot{ + {ID: "shell", Description: target.Description{Name: "Shell", Summary: "Shell details"}, Available: true, State: target.Disabled}, + {ID: "apt", Description: target.Description{Name: "APT", Summary: "APT details"}, Available: true, State: target.Enabled}, + }, + } +} + +func applyOperationCommand(t *testing.T, model Model, command tea.Cmd) (Model, tea.Cmd) { + t.Helper() + if command == nil { + t.Fatal("operation command is nil") + } + updated, next := model.Update(command()) + return updated.(Model), next +} + +func TestCompleteModelKeepsDashboardAndHelpBrowsableWhileMutationIsSerialized(t *testing.T) { + updates := make(chan app.OperationUpdate, 2) + 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) + 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}}} + model, command = applyOperationCommand(t, model, command) + assertUsableFrame(t, model.View(), 120, 36, "Applying", "Writing Shell configuration") + + model = updateKey(model, "down") + if model.selectedTargetID != "apt" { + t.Fatalf("selection while busy = %q, want apt", model.selectedTargetID) + } + model = updateKey(model, "tab") + model = updateKey(model, "down") + model = updateKey(model, "?") + if model.page != helpPage || !strings.Contains(model.View(), "proxyctl Help") { + t.Fatalf("Help was not reachable while mutation ran:\n%s", model.View()) + } + model = updateKey(model, "q") + if model.page != mainPage { + t.Fatal("q did not return from Help while mutation ran") + } + + updated, blocked := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if blocked != nil || !strings.Contains(model.View(), "Wait for the current mutation to finish") || len(module.intents) != 1 { + t.Fatalf("conflicting mutation was not explained: intents=%#v\n%s", module.intents, model.View()) + } +} + +func TestCompleteModelPreservesExpandableCurrentErrorAndFocusesFirstFailedTarget(t *testing.T) { + failure := make(chan app.OperationUpdate, 1) + refresh := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{failure, refresh}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + 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}}} + close(failure) + model, _ = applyOperationCommand(t, model, command) + if model.selectedTargetID != "apt" { + t.Fatalf("first failed Target was not selected: %q", model.selectedTargetID) + } + view := model.View() + if !strings.Contains(view, "Current error: Active Profile changed with one Target failure") || strings.Contains(view, "permission denied") { + t.Fatalf("collapsed current error is wrong:\n%s", view) + } + model = updateKey(model, "e") + if !strings.Contains(model.View(), "apt write failed: permission denied") { + t.Fatalf("current error did not expand:\n%s", model.View()) + } + + 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"}} + 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:") { + t.Fatalf("acknowledged error remained visible:\n%s", model.View()) + } +} + +func TestCompleteModelUsesStructuredConfirmationAndSafeCancellation(t *testing.T) { + confirmation := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{confirmation}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + + 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}} + close(confirmation) + model, _ = applyOperationCommand(t, model, command) + view := model.View() + assertUsableFrame(t, view, 120, 36, "Confirm Docker restart", "Running work may be disrupted", "Enter/n/Esc: Cancel") + + 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) + } +} + +func TestCompleteModelVisiblyDefersCtrlCFromHelpAndStartsNoNewWork(t *testing.T) { + updates := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{updates}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + updated, operation := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + model = updateKey(model, "?") + + updated, quit := model.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) + model = updated.(Model) + 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()) + } + for _, key := range []tea.KeyMsg{tea.KeyMsg{Type: tea.KeySpace}, keyMessage("r"), keyMessage("p")} { + updated, command := model.Update(key) + model = updated.(Model) + if command != nil || len(module.intents) != 1 { + t.Fatalf("new work began after exit request: intents=%#v", module.intents) + } + } + + updates <- app.OperationUpdate{Snapshot: module.snapshot, Phase: app.OperationSucceeded, + Status: app.OperationStatus{Kind: app.StatusSuccess, Summary: "Shell Target is now Enabled", TargetID: "shell"}} + close(updates) + model, quit = applyOperationCommand(t, model, operation) + if quit == nil { + t.Fatal("safe completion did not queue exit") + } + if _, ok := quit().(tea.QuitMsg); !ok { + t.Fatal("safe completion did not return tea.Quit") + } +} diff --git a/internal/tui/pages.go b/internal/tui/pages.go index 9e71d3c..873c099 100644 --- a/internal/tui/pages.go +++ b/internal/tui/pages.go @@ -22,10 +22,13 @@ 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.applying { - footer = "y: confirm n/Esc: cancel" + if model.targetConfirm && !model.operationActive { + footer = "y: confirm Enter/n/Esc: Cancel" } - return terminalPage{ + if model.currentError != nil { + footer += "\ne: expand/collapse current error x: acknowledge error" + } + page := terminalPage{ header: header, footer: footer, dashboard: &dashboardRenderState{ @@ -38,21 +41,50 @@ func (p terminalPresentation) mainView(model presentationState, width, height in progressTarget: model.progressTarget, }, } + if model.confirmation != nil { + var details strings.Builder + details.WriteString(p.theme.section.Render(model.confirmation.Title)) + if model.confirmation.Summary != "" { + fmt.Fprintf(&details, "\n%s", model.confirmation.Summary) + } + for _, detail := range model.confirmation.Details { + fmt.Fprintf(&details, "\n• %s", detail) + } + page.dashboard = nil + page.primary = details.String() + page.context = "Cancellation is selected by default." + } + return page } func (p terminalPresentation) dashboardFooter(model presentationState, catalog targetCatalog) string { + if model.exitDeferred { + return "Exit pending; browsing remains safe while the current mutation completes" + } visibility := "u: show unavailable" if model.showUnavailable { visibility = "u: hide unavailable" } if model.dashboardFocus == targetDetailsPane { - return "↑/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/Esc: 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 { - actions = "Space: toggle " + actions + if model.operationActive { + actions = "Space: disabled (mutation in progress) " + actions + } else { + actions = "Space: toggle " + actions + } } - return actions + "\np: manage Proxy Profiles r: refresh ?: help q/Esc: quit" + footer := actions + "\np: manage Proxy Profiles r: refresh ?: help q/Esc: quit" + if model.operationActive { + footer = actions + "\n?: help q: defer safe exit; new mutations disabled" + } + return footer } func (p terminalPresentation) helpView(model presentationState) terminalPage { diff --git a/internal/tui/presentation.go b/internal/tui/presentation.go index f6ba6fd..801fa5d 100644 --- a/internal/tui/presentation.go +++ b/internal/tui/presentation.go @@ -66,7 +66,14 @@ type presentationState struct { confirm bool targetConfirm bool profileConfirm bool + confirmation *app.Confirmation status string + operationStatus app.OperationStatus + currentError *app.OperationStatus + errorExpanded bool + exitDeferred bool + operationActive bool + operationKind app.IntentKind applying bool profileTransition bool progressTarget string @@ -115,7 +122,7 @@ func (p terminalPresentation) Render(state presentationState) string { default: page = p.mainView(state, width, height) } - page.status = state.status + page.status = renderStatus(state) page = redactTerminalPage(page) // Dashboard composition retains structured Target metadata until pane sizes // are known. Redact the fully composed frame as the final presentation @@ -123,6 +130,25 @@ func (p terminalPresentation) Render(state presentationState) string { return profile.RedactText(p.renderShell(page, width, height)) } +func renderStatus(state presentationState) string { + var lines []string + if state.exitDeferred { + lines = append(lines, "Exit requested — waiting for the current mutation to finish safely") + } else if state.operationStatus.Summary != "" { + lines = append(lines, state.operationStatus.Summary) + } else if state.status != "" { + 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 + } + lines = append(lines, errorText) + } + return strings.Join(lines, "\n") +} + func redactTerminalPage(page terminalPage) terminalPage { page.header = profile.RedactText(page.header) page.primary = profile.RedactText(page.primary) diff --git a/internal/tui/progressive_detection_test.go b/internal/tui/progressive_detection_test.go index 3dec929..bfc7d6f 100644 --- a/internal/tui/progressive_detection_test.go +++ b/internal/tui/progressive_detection_test.go @@ -32,6 +32,23 @@ func (m *progressiveDetectionModule) BeginTargetDetection() (app.Snapshot, <-cha } 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} +} func TestCompleteModelStreamsStartupAndRefreshByTargetWithoutMovingSelection(t *testing.T) { startup := make(chan app.TargetDetectionUpdate, 4) diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index 526b8a5..a6361bb 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -25,6 +25,28 @@ func (m *targetModule) Dispatch(intent app.Intent) app.Result { m.snapshot = m.result.Snapshot return m.result } +func (m *targetModule) 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, 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} + } + close(updates) + return app.OperationStream{Snapshot: m.snapshot, Updates: updates} +} func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing.T) { module := &targetModule{snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{ @@ -40,7 +62,7 @@ func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - if command == nil || !strings.Contains(model.View(), "Applying") || len(module.intents) != 0 { + if command == nil || !strings.Contains(model.View(), "Applying") || len(module.intents) != 1 { t.Fatalf("operation did not expose applying state before effect:\n%s", model.View()) } message := command() @@ -113,7 +135,7 @@ func TestRefreshKeyRunsAsynchronouslyAndRendersRefreshingThenCompleted(t *testin model := NewModel(module) updated, command := model.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'r'}}) model = updated.(Model) - if command == nil || len(module.intents) != 0 || !strings.Contains(model.View(), "Refreshing Target availability and state") { + if command == nil || len(module.intents) != 1 || !strings.Contains(model.View(), "Checking Target availability and state") { t.Fatalf("refresh did not start asynchronously: intents %#v\n%s", module.intents, model.View()) } message := command() @@ -141,19 +163,17 @@ func TestBusyActivationBlocksPageEntryAndEveryNewOperation(t *testing.T) { t.Fatalf("activation did not start exactly once: command=%v intents=%#v", command != nil, module.intents) } - for _, key := range []string{"p", "?", "r", " "} { + 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("busy activation accepted %q: page=%v command=%v intents=%#v", key, model.page, blocked != nil, module.intents) } } - for _, key := range []tea.KeyMsg{keyMessage("q"), {Type: tea.KeyEsc}, {Type: tea.KeyCtrlC}} { - updated, quit := model.Update(key) - deferred := updated.(Model) - if quit != nil || !deferred.quitRequested || len(module.intents) != 1 { - t.Fatalf("busy activation did not defer quit %q: command=%v requested=%v intents=%#v", key.String(), quit != nil, deferred.quitRequested, module.intents) - } + 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) } // Even if a Profiles-page activation key is delivered while the transition @@ -179,25 +199,17 @@ func TestBusyRefreshBlocksTogglePageEntryRefreshAndActivation(t *testing.T) { t.Fatal("refresh did not start") } - for _, key := range []string{" ", "p", "?", "r"} { + for _, key := range []string{" ", "p", "r"} { updated, blocked := model.Update(keyMessage(key)) model = updated.(Model) - if blocked != nil || model.page != mainPage || len(module.intents) != 0 { + if blocked != nil || model.page != mainPage || len(module.intents) != 1 { t.Fatalf("busy refresh accepted %q: page=%v command=%v intents=%#v", key, model.page, blocked != nil, module.intents) } } - for _, key := range []tea.KeyMsg{keyMessage("q"), {Type: tea.KeyEsc}, {Type: tea.KeyCtrlC}} { - updated, quit := model.Update(key) - deferred := updated.(Model) - if quit != nil || !deferred.quitRequested || len(module.intents) != 0 { - t.Fatalf("busy refresh did not defer quit %q: command=%v requested=%v intents=%#v", key.String(), quit != nil, deferred.quitRequested, module.intents) - } - } - - model.page = profilesPage - updated, blocked := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) - if blocked != nil || len(module.intents) != 0 { - t.Fatalf("busy refresh started activation: command=%v intents=%#v", blocked != nil, module.intents) + updated, quit := model.Update(keyMessage("q")) + model = updated.(Model) + if quit != nil || !model.quitRequested || len(module.intents) != 1 { + t.Fatalf("busy refresh did not defer quit: command=%v intents=%#v", quit != nil, module.intents) } } @@ -254,6 +266,7 @@ func TestProfileConfirmationQuitKeysCancelWithoutLeavingBusyState(t *testing.T) } 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") @@ -262,76 +275,6 @@ func TestProfileConfirmationQuitKeysCancelWithoutLeavingBusyState(t *testing.T) } } -func TestSingleOperationGuardRejectsMismatchedCompletionAndPreventsOverlap(t *testing.T) { - initial := app.Snapshot{Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Disabled}}} - refreshed := app.Snapshot{ActiveProfile: "fresh", Targets: initial.Targets} - module := &targetModule{snapshot: initial, result: app.Result{Snapshot: refreshed}} - model := NewModel(module) - updated, refreshCommand := model.Update(keyMessage("r")) - model = updated.(Model) - updated, command := model.Update(keyMessage("q")) - model = updated.(Model) - if command != nil || !model.quitRequested { - t.Fatal("quit was not deferred during refresh") - } - - updated, command = model.Update(targetOperationDone{result: app.Result{Snapshot: app.Snapshot{ActiveProfile: "stale"}}}) - model = updated.(Model) - if command != nil || !model.refreshing || !model.busy() || model.snapshot.ActiveProfile != "" { - t.Fatal("completion from a non-current operation cleared or corrupted refresh state") - } - updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace}) - model = updated.(Model) - if command != nil { - t.Fatal("Target operation overlapped the in-flight refresh") - } - - refreshCompletion := refreshCommand() - updated, command = model.Update(refreshCompletion) - model = updated.(Model) - if command == nil || !model.busy() || model.snapshot.ActiveProfile != "fresh" { - t.Fatal("current refresh completion did not apply fresh state and enter terminal deferred-quit state") - } - if _, ok := command().(tea.QuitMsg); !ok { - t.Fatal("current completion did not return tea.Quit") - } -} - -func TestBusyQuitWaitsForLongRunningTargetMutationCompletion(t *testing.T) { - for _, quitKey := range []tea.KeyMsg{keyMessage("q"), {Type: tea.KeyEsc}, {Type: tea.KeyCtrlC}} { - t.Run(quitKey.String(), func(t *testing.T) { - initial := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Disabled}}} - completed := app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Enabled}}} - module := &targetModule{snapshot: initial, result: app.Result{Snapshot: completed, Notice: "Shell Target is now Enabled"}} - model := NewModel(module) - updated, operation := model.Update(tea.KeyMsg{Type: tea.KeySpace}) - model = updated.(Model) - if operation == nil || len(module.intents) != 0 { - t.Fatal("Target mutation did not start asynchronously") - } - - updated, quit := model.Update(quitKey) - model = updated.(Model) - if quit != nil || !model.quitRequested || len(module.intents) != 0 { - t.Fatalf("quit was not deferred before mutation completion: command=%v requested=%v intents=%d", quit != nil, model.quitRequested, len(module.intents)) - } - - completion := operation() - if len(module.intents) != 1 { - t.Fatal("deferred quit abandoned the in-flight mutation") - } - updated, quit = model.Update(completion) - model = updated.(Model) - if quit == nil || !model.busy() || model.snapshot.Targets[0].State != target.Enabled { - t.Fatalf("mutation completion was not consumed before terminal deferred quit: busy=%v snapshot=%#v", model.busy(), model.snapshot) - } - if _, ok := quit().(tea.QuitMsg); !ok { - t.Fatal("completion did not return tea.Quit") - } - }) - } -} - func TestDeferredQuitIsTerminalAfterTargetConfirmationCompletion(t *testing.T) { initial := app.Snapshot{ ActiveProfile: "office", @@ -504,6 +447,45 @@ func (m *progressiveTargetModule) DispatchProgress(intent app.Intent) <-chan app 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} +} 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}}} @@ -572,8 +554,9 @@ func TestTargetFailurePreservesTruthfulStateAndRendersRedactedError(t *testing.T model = updated.(Model) updated, _ = model.Update(command()) model = updated.(Model) + model = updateKey(model, "e") view := model.View() - if !strings.Contains(view, "Enabled") || !strings.Contains(view, "Error:") || !strings.Contains(view, "REDACTED") { + if !strings.Contains(view, "Enabled") || !strings.Contains(view, "Current error:") || !strings.Contains(view, "REDACTED") { t.Fatalf("failed operation rendering:\n%s", view) } }