fix: refresh mutations and defer busy quit

This commit is contained in:
proxyctl coordinator
2026-07-17 10:28:40 +08:00
parent f6c6ead325
commit eb8932a319
5 changed files with 182 additions and 31 deletions
+12
View File
@@ -153,6 +153,18 @@ func (a *Application) dispatch(intent Intent, publish func(Result)) Result {
}
defer unlock()
}
// A different process may have changed the persisted Active Profile since
// this Application was constructed. Reload only after the global mutation
// lock is held, before selecting a Profile or invoking a Target. Load is
// deliberately separate from Store.Mutate so no Profile-store lock is nested.
fresh, err := a.store.Load()
if err != nil {
return Result{Snapshot: a.Snapshot(), Err: fmt.Errorf("reload Profile state for mutation: %w", err)}
}
a.state = fresh
if intent.Kind == SetTargetDesired || intent.Kind == ActivateProfile {
a.refreshInspections()
}
if intent.Kind == SetTargetDesired {
return a.setTargetDesired(intent)
}
+47
View File
@@ -10,6 +10,7 @@ import (
"proxyctl/internal/app"
"proxyctl/internal/dockerclient"
"proxyctl/internal/operationlock"
"proxyctl/internal/profile"
"proxyctl/internal/shell"
"proxyctl/internal/target"
@@ -56,6 +57,52 @@ func (t *coordinatedTarget) SetDesired(_ context.Context, enabled bool, active *
return target.OperationResult{Inspection: target.Inspection{State: t.state, Evidence: "matches " + active.Name}}
}
func TestSecondApplicationUsesPersistedActiveProfileChangedByFirstApplication(t *testing.T) {
root := t.TempDir()
storePath := filepath.Join(root, "profiles.json")
bootstrap, err := app.New(profile.NewFileStore(storePath))
if err != nil {
t.Fatal(err)
}
for _, name := range []string{"old", "new"} {
result := bootstrap.Dispatch(app.Intent{Kind: app.CreateProfile, Profile: profile.Profile{Name: name, Mode: profile.SingleMode, SingleURL: "http://" + name + ".example:8080"}})
if result.Err != nil {
t.Fatalf("create %s Profile: %v", name, result.Err)
}
}
if result := bootstrap.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "old"}); result.Err != nil {
t.Fatalf("activate old Profile: %v", result.Err)
}
firstTarget := &coordinatedTarget{id: "shell", state: target.Disabled}
secondTarget := &coordinatedTarget{id: "shell", state: target.Disabled}
first, err := app.NewWithTargetsAndLock(profile.NewFileStore(storePath), []target.Target{firstTarget}, operationlock.New(filepath.Join(root, "operation.lock")))
if err != nil {
t.Fatal(err)
}
second, err := app.NewWithTargetsAndLock(profile.NewFileStore(storePath), []target.Target{secondTarget}, operationlock.New(filepath.Join(root, "operation.lock")))
if err != nil {
t.Fatal(err)
}
if got := second.Snapshot().ActiveProfile; got != "old" {
t.Fatalf("second Application starting Active Profile = %q, want old", got)
}
if result := first.Dispatch(app.Intent{Kind: app.ActivateProfile, Name: "new"}); result.Err != nil {
t.Fatalf("first Application activates new Profile: %v", result.Err)
}
result := second.Dispatch(app.Intent{Kind: app.SetTargetDesired, TargetID: "shell", Enabled: true})
if result.Err != nil {
t.Fatalf("second Application enables Target: %v", result.Err)
}
if secondTarget.appliedName != "new" {
t.Fatalf("second Application applied stale Profile %q, want freshly persisted Active Profile new", secondTarget.appliedName)
}
if result.Snapshot.ActiveProfile != "new" {
t.Fatalf("second Application snapshot Active Profile = %q, want new", result.Snapshot.ActiveProfile)
}
}
func TestActiveProfileSwitchCoordinatesEnabledTargetsWithoutRollback(t *testing.T) {
first := &coordinatedTarget{id: "first", state: target.Disabled}
second := &coordinatedTarget{id: "second", state: target.Disabled, failName: "new"}
+2 -2
View File
@@ -104,8 +104,8 @@ func TestApplicationReloadsStateAfterCommittedDurabilityError(t *testing.T) {
if !errors.As(result.Err, &committed) {
t.Fatalf("mutation error = %v, want committed durability error", result.Err)
}
if store.loadCalls != 2 {
t.Fatalf("Load calls = %d, want startup plus post-commit reload", store.loadCalls)
if store.loadCalls != 3 {
t.Fatalf("Load calls = %d, want startup, pre-mutation, and post-commit reloads", store.loadCalls)
}
if len(result.Snapshot.Profiles) != 1 || result.Snapshot.Profiles[0].Name != "committed" {
t.Fatalf("Application snapshot after committed error = %#v", result.Snapshot)
+30 -5
View File
@@ -41,6 +41,7 @@ type Model struct {
refreshing bool
profileTransition bool
progressTarget string
quitRequested bool
}
type targetOperationDone struct {
@@ -76,10 +77,18 @@ func (m Model) Init() tea.Cmd { return nil }
// 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.applying || m.refreshing || m.profileTransition || m.pendingActivation.Kind != 0
return m.operationInFlight() || m.pendingActivation.Kind != 0
}
func (m Model) operationInFlight() bool {
return m.applying || m.refreshing || m.profileTransition
}
func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) {
if key, ok := message.(tea.KeyMsg); ok && m.operationInFlight() && isQuitKey(key) {
m.quitRequested = true
return m, nil
}
if completed, ok := message.(refreshDone); ok {
if !m.refreshing {
return m, nil
@@ -99,7 +108,7 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) {
} else if m.selected >= targetCount {
m.selected = targetCount - 1
}
return m, nil
return m, m.deferredQuit()
}
if progress, ok := message.(profileOperationProgress); ok {
if !m.profileTransition {
@@ -120,7 +129,7 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) {
intent := m.pendingActivation
m.profileConfirm = &intent
m.status = progress.result.Confirmation
return m, nil
return m, m.deferredQuit()
}
if progress.result.Err != nil {
m.status = "\x1b[31mError: " + progress.result.Err.Error() + "\x1b[0m"
@@ -132,7 +141,7 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) {
m.profileTransition = false
m.progressTarget = ""
m.pendingActivation = app.Intent{}
return m, nil
return m, m.deferredQuit()
}
return m, waitForProfileProgress(progress.updates)
}
@@ -156,7 +165,7 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) {
m.targetConfirm = nil
m.status = completed.result.Notice
}
return m, nil
return m, m.deferredQuit()
}
key, ok := message.(tea.KeyMsg)
if !ok {
@@ -179,6 +188,22 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) {
}
}
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.busy() {
switch key.String() {
+91 -24
View File
@@ -147,13 +147,11 @@ func TestBusyActivationBlocksPageEntryAndEveryNewOperation(t *testing.T) {
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}} {
_, quit := model.Update(key)
if quit == nil {
t.Fatalf("busy activation changed quit behavior for %q", key.String())
}
if _, ok := quit().(tea.QuitMsg); !ok || len(module.intents) != 1 {
t.Fatalf("busy activation quit %q triggered an operation: intents=%#v", key.String(), 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)
}
}
@@ -187,13 +185,11 @@ func TestBusyRefreshBlocksTogglePageEntryRefreshAndActivation(t *testing.T) {
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}} {
_, quit := model.Update(key)
if quit == nil {
t.Fatalf("busy refresh changed quit behavior for %q", key.String())
}
if _, ok := quit().(tea.QuitMsg); !ok || len(module.intents) != 0 {
t.Fatalf("busy refresh quit %q triggered an operation: intents=%#v", key.String(), 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)
}
}
@@ -234,17 +230,54 @@ func TestPendingActivationRemainsBusyUntilConfirmationIsResolved(t *testing.T) {
}
}
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"}
updated, command = model.Update(tea.KeyMsg{Type: tea.KeyEnter})
if command == nil {
t.Fatal("confirmation cancellation prevented a later activation")
}
})
}
}
func TestSingleOperationGuardRejectsMismatchedCompletionAndPreventsOverlap(t *testing.T) {
initial := app.Snapshot{Targets: []app.TargetSnapshot{{ID: "shell", Description: target.Description{Name: "Shell"}, Available: true, State: target.Disabled}}}
module := &targetModule{snapshot: initial, result: app.Result{Snapshot: initial}}
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(targetOperationDone{result: app.Result{Snapshot: app.Snapshot{}}})
updated, command := model.Update(keyMessage("q"))
model = updated.(Model)
if command != nil || !model.refreshing || !model.busy() {
t.Fatal("completion from a non-current operation cleared refresh state")
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)
@@ -255,12 +288,46 @@ func TestSingleOperationGuardRejectsMismatchedCompletionAndPreventsOverlap(t *te
refreshCompletion := refreshCommand()
updated, command = model.Update(refreshCompletion)
model = updated.(Model)
if command != nil || model.busy() {
t.Fatal("current refresh completion did not clear busy state")
if command == nil || model.busy() || model.snapshot.ActiveProfile != "fresh" {
t.Fatal("current refresh completion did not apply fresh state and trigger deferred quit")
}
updated, command = model.Update(tea.KeyMsg{Type: tea.KeySpace})
if command == nil {
t.Fatal("new operation was not allowed after the prior completion was consumed")
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 deferred quit: busy=%v snapshot=%#v", model.busy(), model.snapshot)
}
if _, ok := quit().(tea.QuitMsg); !ok {
t.Fatal("completion did not return tea.Quit")
}
})
}
}