diff --git a/internal/tui/contextual_actions_test.go b/internal/tui/contextual_actions_test.go index 7d74fe8..da908c4 100644 --- a/internal/tui/contextual_actions_test.go +++ b/internal/tui/contextual_actions_test.go @@ -149,8 +149,7 @@ func TestCompleteModelRestoreOwnershipLossFailsTruthfully(t *testing.T) { if command == nil { t.Fatal("Restore Original did not start") } - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) if len(module.intents) != 1 || module.intents[0].Kind != app.RestoreOriginal || !strings.Contains(model.View(), "Current error") || !strings.Contains(model.View(), "Drifted") { t.Fatalf("ownership-loss result: intents=%#v\n%s", module.intents, model.View()) } diff --git a/internal/tui/foundation_test.go b/internal/tui/foundation_test.go index 598e6f4..3f1d5d1 100644 --- a/internal/tui/foundation_test.go +++ b/internal/tui/foundation_test.go @@ -208,8 +208,7 @@ func TestCompleteModelRedactsApplicationErrorsBeforeRendering(t *testing.T) { model := resizedModel(t, module, themeModeNoColor, 80, 24) updated, cmd := model.Update(keyMessage("r")) model = updated.(Model) - updated, _ = model.Update(cmd()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, cmd) model = updateKey(model, "e") view := model.View() if strings.Contains(view, secret) || !strings.Contains(view, "https://bob:REDACTED@proxy.example:8080") { @@ -228,8 +227,7 @@ func TestCompleteModelNarrowShellKeepsLastProductionTargetStatusAndActionsVisibl updated, refresh := model.Update(keyMessage("r")) model = updated.(Model) - updated, _ = model.Update(refresh()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, refresh) assertUsableFrame(t, model.View(), 80, 24, "> ! Drifted Docker daemon", diff --git a/internal/tui/model.go b/internal/tui/model.go index 553071b..67ceddc 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -92,7 +92,7 @@ func targetsChecking(snapshot app.Snapshot) bool { return false } -func (m Model) Init() tea.Cmd { return m.operation.wait() } +func (m Model) Init() tea.Cmd { return m.operation.commands() } // 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 diff --git a/internal/tui/operation_coordination_test.go b/internal/tui/operation_coordination_test.go index 434d17c..f7e281e 100644 --- a/internal/tui/operation_coordination_test.go +++ b/internal/tui/operation_coordination_test.go @@ -4,6 +4,7 @@ import ( "errors" "strings" "testing" + "time" "github.com/charmbracelet/bubbles/spinner" tea "github.com/charmbracelet/bubbletea" @@ -42,15 +43,53 @@ func operationSnapshot() app.Snapshot { } } +// applyOperationCommand drives the same command shape Bubble Tea drives in +// production. An operation starts the stream wait and spinner tick as an +// independent Batch; tests select the stream result without coupling its +// completion to animation timing. 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()) + message, ok := operationStreamMessage(command()) + if !ok { + t.Fatal("operation command produced no operation-stream message") + } + updated, next := model.Update(message) return updated.(Model), next } +func operationStreamMessage(message tea.Msg) (tea.Msg, bool) { + switch typed := message.(type) { + case operationProgress: + return typed, true + case tea.BatchMsg: + for _, command := range typed { + if command == nil { + continue + } + if message, ok := operationStreamMessage(command()); ok { + return message, true + } + } + case spinner.TickMsg: + // Spinner ticks have their own production command chain. Applying one in + // this helper would schedule animation work unrelated to stream progress. + return nil, false + } + return nil, false +} + +func TestCompleteModelSpinnerAnimationHasDeterministicCadence(t *testing.T) { + if got, want := applyingSpinner.FPS, 90*time.Millisecond; got != want { + t.Fatalf("spinner cadence = %s, want %s", got, want) + } + if got, want := strings.Join(applyingSpinner.Frames, ""), "|/-\\"; got != want { + t.Fatalf("spinner frames = %q, want %q", got, want) + } +} + func TestCompleteModelUsesAnimatedBubblesSpinnerOnlyWhileOperationRuns(t *testing.T) { updates := make(chan app.OperationUpdate) module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{updates}} @@ -61,6 +100,11 @@ func TestCompleteModelUsesAnimatedBubblesSpinnerOnlyWhileOperationRuns(t *testin if command == nil || !strings.Contains(model.View(), "| Applying") { t.Fatalf("operation did not start the Bubbles spinner:\n%s", model.View()) } + message := command() + batch, ok := message.(tea.BatchMsg) + if !ok || len(batch) != 2 { + t.Fatalf("operation command = %T with %d children, want independent stream and spinner Batch", message, len(batch)) + } updated, next := model.Update(spinner.TickMsg{ID: model.operation.spinner.ID()}) model = updated.(Model) if next == nil || !strings.Contains(model.View(), "/ Applying") { @@ -184,6 +228,70 @@ func TestCompleteModelReadOnlyRefreshDoesNotClearTargetMutationErrorForSameTarge } } +func TestCompleteModelFailedRefreshDoesNotReplaceTargetMutationError(t *testing.T) { + mutationFailure := make(chan app.OperationUpdate, 1) + refreshFailure := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{mutationFailure, refreshFailure}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + mutationFailure <- app.FailedOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Shell Target update failed", TargetID: "shell"}, nil, "shell", errors.New("mutation permission denied")) + close(mutationFailure) + model, _ = applyOperationCommand(t, model, command) + + updated, command = model.Update(keyMessage("r")) + model = updated.(Model) + refreshFailure <- app.FailedOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Some Target checks failed", TargetID: "apt"}, nil, "apt", errors.New("refresh inspection failed")) + close(refreshFailure) + model, _ = applyOperationCommand(t, model, command) + + view := model.View() + if !strings.Contains(view, "Current error: Shell Target update failed") { + t.Fatalf("failed Refresh Targets replaced the current mutation error:\n%s", view) + } + if !strings.Contains(view, "Status: Some Target checks failed") { + t.Fatalf("failed Refresh Targets status was not shown separately:\n%s", view) + } + model = updateKey(model, "e") + view = model.View() + if !strings.Contains(view, "mutation permission denied") || strings.Contains(view, "refresh inspection failed") { + t.Fatalf("expanded current error did not preserve the mutation detail:\n%s", view) + } +} + +func TestCompleteModelRelatedTargetMutationFailureReplacesErrorForSameTarget(t *testing.T) { + firstFailure := make(chan app.OperationUpdate, 1) + relatedFailure := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{firstFailure, relatedFailure}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + firstFailure <- app.FailedOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Initial Shell Target failure", TargetID: "shell"}, nil, "shell", errors.New("initial detail")) + close(firstFailure) + model, _ = applyOperationCommand(t, model, command) + + updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + relatedFailure <- app.FailedOperationUpdate(module.snapshot, + app.OperationStatus{Summary: "Latest Shell Target failure", TargetID: "shell"}, nil, "shell", errors.New("latest detail")) + close(relatedFailure) + model, _ = applyOperationCommand(t, model, command) + + view := model.View() + if !strings.Contains(view, "Current error: Latest Shell Target failure") || strings.Contains(view, "Initial Shell Target failure") { + t.Fatalf("related Target mutation failure did not replace the prior error:\n%s", view) + } + model = updateKey(model, "e") + if view := model.View(); !strings.Contains(view, "latest detail") || strings.Contains(view, "initial detail") { + t.Fatalf("related Target mutation failure did not replace the prior detail:\n%s", view) + } +} + func TestCompleteModelRelatedTargetMutationClearsErrorForSameTarget(t *testing.T) { failure := make(chan app.OperationUpdate, 1) resolution := make(chan app.OperationUpdate, 1) diff --git a/internal/tui/operation_coordinator.go b/internal/tui/operation_coordinator.go index 2627154..b1c5517 100644 --- a/internal/tui/operation_coordinator.go +++ b/internal/tui/operation_coordinator.go @@ -52,28 +52,13 @@ func newOperationCoordinator() operationCoordinator { } } -// waitForOperation races the stream against the real Bubbles tick command. -// Only the stream is read directly, so a losing tick cannot consume an update. -func waitForOperation(updates <-chan app.OperationUpdate, tick tea.Cmd) tea.Cmd { +func waitForOperation(updates <-chan app.OperationUpdate) tea.Cmd { return func() tea.Msg { - if tick == nil { - update, ok := <-updates - if !ok { - return operationProgress{updates: updates, closed: true} - } - return operationProgress{update: update, updates: updates} - } - ticks := make(chan tea.Msg, 1) - go func() { ticks <- tick() }() - select { - case update, ok := <-updates: - if !ok { - return operationProgress{updates: updates, closed: true} - } - return operationProgress{update: update, updates: updates} - case message := <-ticks: - return message + update, ok := <-updates + if !ok { + return operationProgress{updates: updates, closed: true} } + return operationProgress{update: update, updates: updates} } } @@ -81,7 +66,14 @@ func (o operationCoordinator) wait() tea.Cmd { if !o.active || o.updates == nil { return nil } - return waitForOperation(o.updates, o.spinner.Tick) + return waitForOperation(o.updates) +} + +func (o operationCoordinator) commands() tea.Cmd { + if !o.active { + return nil + } + return tea.Batch(o.wait(), o.spinner.Tick) } func (o *operationCoordinator) start(application app.InteractiveModule, intent app.Intent) (app.Snapshot, tea.Cmd, bool) { @@ -99,7 +91,7 @@ func (o *operationCoordinator) start(application app.InteractiveModule, intent a o.progress[intent.TargetID] = app.TargetProgressRunning } o.status = app.OperationStatus{Kind: app.StatusProgress, Summary: operationStartingSummary(intent.Kind)} - return stream.Snapshot, o.wait(), true + return stream.Snapshot, o.commands(), true } func operationStartingSummary(kind app.IntentKind) string { @@ -119,7 +111,7 @@ func (o *operationCoordinator) tick(message spinner.TickMsg) tea.Cmd { } updated, nextTick := o.spinner.Update(message) o.spinner = updated - return waitForOperation(o.updates, nextTick) + return nextTick } func (o *operationCoordinator) apply(update app.OperationUpdate) (firstFailedTargetID string) { @@ -134,9 +126,7 @@ func (o *operationCoordinator) apply(update app.OperationUpdate) (firstFailedTar case app.OperationSucceeded: o.resolveError(o.intent) case app.OperationFailed: - o.currentError = &operationError{status: decoded.Status, detail: errorDetail(decoded.Err), intent: o.intent} - o.errorExpanded = false - o.errorOffset = 0 + o.recordError(&operationError{status: decoded.Status, detail: errorDetail(decoded.Err), intent: o.intent}) firstFailedTargetID = decoded.FirstFailedTargetID case app.OperationConfirmation: intent := o.intent @@ -157,6 +147,15 @@ func errorDetail(err error) string { return err.Error() } +func (o *operationCoordinator) recordError(candidate *operationError) { + if o.currentError != nil && !relatedResolution(o.currentError.intent, candidate.intent) { + return + } + o.currentError = candidate + o.errorExpanded = false + o.errorOffset = 0 +} + func (o *operationCoordinator) resolveError(success app.Intent) { if o.currentError == nil || !relatedResolution(o.currentError.intent, success) { return @@ -196,13 +195,11 @@ func (o *operationCoordinator) finish() { func (o *operationCoordinator) unexpectedClose() { o.status = app.OperationStatus{Kind: app.StatusError, Summary: "Operation ended without a final result"} - o.currentError = &operationError{ + o.recordError(&operationError{ status: o.status, detail: "The Application operation stream closed before publishing a terminal result.", intent: o.intent, - } - o.errorExpanded = false - o.errorOffset = 0 + }) o.finish() } diff --git a/internal/tui/progressive_detection_test.go b/internal/tui/progressive_detection_test.go index 95b4d4d..5b7fb66 100644 --- a/internal/tui/progressive_detection_test.go +++ b/internal/tui/progressive_detection_test.go @@ -156,8 +156,7 @@ func cloneDetectionSnapshot(snapshot app.Snapshot) app.Snapshot { func applyDetectionCommand(t *testing.T, model Model, command tea.Cmd) (Model, tea.Cmd) { t.Helper() - updated, next := model.Update(command()) - return updated.(Model), next + return applyOperationCommand(t, model, command) } func assertOrder(t *testing.T, view string, values ...string) { diff --git a/internal/tui/render_hardening_test.go b/internal/tui/render_hardening_test.go index 5a70ec8..8b4a24f 100644 --- a/internal/tui/render_hardening_test.go +++ b/internal/tui/render_hardening_test.go @@ -152,8 +152,7 @@ func TestCompleteModelLightDarkAndNoColorRetainEquivalentSemantics(t *testing.T) model = updated.(Model) model = updateKey(model, "tab") progressFrame := model.View() - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) model = updateKey(model, "e") views[mode] = progressFrame + "--- operation completed ---\n" + model.View() for _, semantic := range []string{"[focused]", "Disabled", "Drifted", "Applying", "mutations disabled", "Current error:", "REDACTED", "Action: Enable"} { @@ -260,8 +259,7 @@ func TestCompleteModelLongFieldsAndDialogsRemainReachableAcrossResizeAtNontermin model := resizedModel(t, module, themeModeNoColor, 80, 24) updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) model = updateKey(model, "e") model = updateKey(model, "pgdown") model = updateKey(model, "pgdown") diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index 6277b01..b58612d 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -58,9 +58,7 @@ func TestTargetPanelShowsOnlyAvailableTargetsAndAppliesAsynchronously(t *testing 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() - updated, _ = model.Update(message) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) if len(module.intents) != 1 || module.intents[0].Kind != app.ApplyProfileToTarget || !strings.Contains(model.View(), "Enabled") { t.Fatalf("completed toggle = %#v\n%s", module.intents, model.View()) } @@ -72,8 +70,7 @@ func TestTargetRestartConfirmationCancelsWithoutConfirmedMutation(t *testing.T) model := NewModel(module) updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) if !strings.Contains(model.View(), "restart immediately") || len(module.intents) != 1 || module.intents[0].Confirm { t.Fatalf("warning = intents %#v\n%s", module.intents, model.View()) } @@ -85,8 +82,7 @@ func TestTargetRestartConfirmationCancelsWithoutConfirmedMutation(t *testing.T) updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) module.result = app.Result{ Snapshot: app.Snapshot{ActiveProfile: "office", Targets: []app.TargetSnapshot{{ID: "docker-daemon", Description: target.Description{Name: "Docker daemon"}, Available: true, State: target.Enabled, PrimaryAction: app.TargetAction{Kind: app.DisableTargetAction, Available: true}}}}, Notice: "Docker daemon Target is now Enabled", @@ -96,7 +92,6 @@ func TestTargetRestartConfirmationCancelsWithoutConfirmedMutation(t *testing.T) if command == nil { t.Fatal("confirmation did not start operation") } - _ = command() if len(module.intents) != 3 || !module.intents[2].Confirm { t.Fatalf("confirmed intents = %#v", module.intents) } @@ -141,12 +136,10 @@ func TestRefreshKeyRunsAsynchronouslyAndRendersRefreshingThenCompleted(t *testin 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() if len(module.intents) != 1 || module.intents[0].Kind != app.RefreshTargets { t.Fatalf("refresh command intents = %#v", module.intents) } - updated, command = model.Update(message) - model = updated.(Model) + model, command = applyOperationCommand(t, model, command) if command != nil || !strings.Contains(model.View(), "External") || !strings.Contains(model.View(), "Refreshed Target availability and state") { t.Fatalf("refresh completion:\n%s", model.View()) } @@ -228,8 +221,7 @@ func TestRefreshClampsSelectionAfterMultipleTargetsDisappear(t *testing.T) { model.selectedTargetID = "four" updated, command := model.Update(keyMessage("r")) model = updated.(Model) - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) if model.selectedTargetID != "one" || !strings.Contains(model.View(), "> ? Unknown One") { t.Fatalf("selection after multi-Target reduction = %q\n%s", model.selectedTargetID, model.View()) } @@ -237,8 +229,7 @@ func TestRefreshClampsSelectionAfterMultipleTargetsDisappear(t *testing.T) { module.result = app.Result{Snapshot: app.Snapshot{}} updated, command = model.Update(keyMessage("r")) model = updated.(Model) - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) if model.selectedTargetID != "" { t.Fatalf("selection with no Available Targets = %q, want empty", model.selectedTargetID) } @@ -323,18 +314,15 @@ func TestActiveProfileTransitionRendersPerTargetProgress(t *testing.T) { if command == nil { t.Fatal("Profile transition did not start progressive operation") } - updated, command := model.Update(command()) - model = updated.(Model) + model, command = applyOperationCommand(t, model, command) if command == nil || !strings.Contains(model.View(), "Applying") || !strings.Contains(model.View(), "Active Profile: new") { t.Fatalf("applying progress:\n%s", model.View()) } - updated, command = model.Update(command()) - model = updated.(Model) + model, command = applyOperationCommand(t, model, command) if command == nil || !strings.Contains(model.View(), "Enabled") || !strings.Contains(model.View(), "update completed") { t.Fatalf("Target completion:\n%s", model.View()) } - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) if !strings.Contains(model.View(), "Activated Profile new") { t.Fatalf("final transition:\n%s", model.View()) } @@ -379,8 +367,7 @@ func TestTargetFailurePreservesTruthfulStateAndRendersRedactedError(t *testing.T model := NewModel(module) updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) model = updated.(Model) - updated, _ = model.Update(command()) - model = updated.(Model) + model, _ = applyOperationCommand(t, model, command) model = updateKey(model, "e") view := model.View() if !strings.Contains(view, "Enabled") || !strings.Contains(view, "Current error:") || !strings.Contains(view, "REDACTED") {