fix(tabs): serialize tab persistence updates
- commit
- 21125498a4ffa11b33cfd29b42d2621947d89637
- author
- brice · · 2026-07-04 02:02 UTC
- parent
- 81e6be1
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +17 -28
M internal/usecase/daemon/daemon.go +2 -13
| @@ -30,7 +30,6 @@ import ( | |||
| 30 | 30 | "errors" | |
| 31 | 31 | "log/slog" | |
| 32 | 32 | "os" | |
| 33 | - | "slices" | |
| 34 | 33 | "sort" | |
| 35 | 34 | "sync" | |
| 36 | 35 | "time" | |
| @@ -125,14 +124,6 @@ type stoppedSession struct { | |||
| 125 | 124 | purging bool | |
| 126 | 125 | } | |
| 127 | 126 | ||
| 128 | - | func (s stoppedSession) same(other stoppedSession) bool { | |
| 129 | - | return s.name == other.name && | |
| 130 | - | s.cwd == other.cwd && | |
| 131 | - | s.createdAt == other.createdAt && | |
| 132 | - | s.purging == other.purging && | |
| 133 | - | slices.Equal(s.tabNames, other.tabNames) | |
| 134 | - | } | |
| 135 | - | ||
| 136 | 127 | type Option func(*Daemon) | |
| 137 | 128 | ||
| 138 | 129 | // WithShell overrides the command (and its args) each session spawns. The | |
| @@ -373,16 +364,14 @@ func (d *Daemon) handleKill(tr ports.Transport, f ports.Frame) { | |||
| 373 | 364 | d.mu.Lock() | |
| 374 | 365 | target := d.findByNameLocked(k.Name) | |
| 375 | 366 | if target == nil { | |
| 376 | - | if stopped, ok := d.stopped[k.Name]; ok { | |
| 367 | + | if _, ok := d.stopped[k.Name]; ok { | |
| 377 | 368 | if err := d.persist.Delete(k.Name); err != nil { | |
| 378 | 369 | d.mu.Unlock() | |
| 379 | 370 | d.log.Warn("deleting persisted stopped session failed", "err", err, "session", k.Name) | |
| 380 | 371 | _ = tr.Send(frameError(ports.ErrInternal, "deleting persisted stopped session failed")) | |
| 381 | 372 | return | |
| 382 | 373 | } | |
| 383 | - | if cur, ok := d.stopped[k.Name]; ok && cur.same(stopped) { | |
| 384 | - | delete(d.stopped, k.Name) | |
| 385 | - | } | |
| 374 | + | delete(d.stopped, k.Name) | |
| 386 | 375 | d.mu.Unlock() | |
| 387 | 376 | return | |
| 388 | 377 | } | |
| @@ -30,7 +30,6 @@ import ( | |||||
| 30 | "errors" | 30 | "errors" | ||
| 31 | "log/slog" | 31 | "log/slog" | ||
| 32 | "os" | 32 | "os" | ||
| 33 | - | "slices" | |||
| 34 | "sort" | 33 | "sort" | ||
| 35 | "sync" | 34 | "sync" | ||
| 36 | "time" | 35 | "time" | ||
| @@ -125,14 +124,6 @@ type stoppedSession struct { | |||||
| 125 | purging bool | 124 | purging bool | ||
| 126 | } | 125 | } | ||
| 127 | 126 | ||||
| 128 | - | func (s stoppedSession) same(other stoppedSession) bool { | |||
| 129 | - | return s.name == other.name && | |||
| 130 | - | s.cwd == other.cwd && | |||
| 131 | - | s.createdAt == other.createdAt && | |||
| 132 | - | s.purging == other.purging && | |||
| 133 | - | slices.Equal(s.tabNames, other.tabNames) | |||
| 134 | - | } | |||
| 135 | - | ||||
| 136 | type Option func(*Daemon) | 127 | type Option func(*Daemon) | ||
| 137 | 128 | ||||
| 138 | // WithShell overrides the command (and its args) each session spawns. The | 129 | // WithShell overrides the command (and its args) each session spawns. The | ||
| @@ -373,16 +364,14 @@ func (d *Daemon) handleKill(tr ports.Transport, f ports.Frame) { | |||||
| 373 | d.mu.Lock() | 364 | d.mu.Lock() | ||
| 374 | target := d.findByNameLocked(k.Name) | 365 | target := d.findByNameLocked(k.Name) | ||
| 375 | if target == nil { | 366 | if target == nil { | ||
| 376 | - | if stopped, ok := d.stopped[k.Name]; ok { | 367 | + | if _, ok := d.stopped[k.Name]; ok { |
| 377 | if err := d.persist.Delete(k.Name); err != nil { | 368 | if err := d.persist.Delete(k.Name); err != nil { | ||
| 378 | d.mu.Unlock() | 369 | d.mu.Unlock() | ||
| 379 | d.log.Warn("deleting persisted stopped session failed", "err", err, "session", k.Name) | 370 | d.log.Warn("deleting persisted stopped session failed", "err", err, "session", k.Name) | ||
| 380 | _ = tr.Send(frameError(ports.ErrInternal, "deleting persisted stopped session failed")) | 371 | _ = tr.Send(frameError(ports.ErrInternal, "deleting persisted stopped session failed")) | ||
| 381 | return | 372 | return | ||
| 382 | } | 373 | } | ||
| 383 | - | if cur, ok := d.stopped[k.Name]; ok && cur.same(stopped) { | 374 | + | delete(d.stopped, k.Name) |
| 384 | - | delete(d.stopped, k.Name) | |||
| 385 | - | } | |||
| 386 | d.mu.Unlock() | 375 | d.mu.Unlock() | ||
| 387 | return | 376 | return | ||
| 388 | } | 377 | } | ||
M internal/usecase/daemon/picker.go +1 -3
| @@ -449,9 +449,7 @@ func (d *Daemon) killPickerTarget(target picker.Target) { | |||
| 449 | 449 | d.log.Warn("deleting persisted stopped session failed", "err", err, "session", target.Name) | |
| 450 | 450 | return | |
| 451 | 451 | } | |
| 452 | - | if cur, ok := d.stopped[target.Name]; ok && cur.same(stopped) { | |
| 453 | - | delete(d.stopped, target.Name) | |
| 454 | - | } | |
| 452 | + | delete(d.stopped, target.Name) | |
| 455 | 453 | } | |
| 456 | 454 | d.mu.Unlock() | |
| 457 | 455 | return | |
| @@ -449,9 +449,7 @@ func (d *Daemon) killPickerTarget(target picker.Target) { | |||||
| 449 | d.log.Warn("deleting persisted stopped session failed", "err", err, "session", target.Name) | 449 | d.log.Warn("deleting persisted stopped session failed", "err", err, "session", target.Name) | ||
| 450 | return | 450 | return | ||
| 451 | } | 451 | } | ||
| 452 | - | if cur, ok := d.stopped[target.Name]; ok && cur.same(stopped) { | 452 | + | delete(d.stopped, target.Name) |
| 453 | - | delete(d.stopped, target.Name) | |||
| 454 | - | } | |||
| 455 | } | 453 | } | ||
| 456 | d.mu.Unlock() | 454 | d.mu.Unlock() | ||
| 457 | return | 455 | return | ||
M internal/usecase/daemon/session.go +12 -10
| @@ -28,6 +28,7 @@ import ( | |||
| 28 | 28 | "context" | |
| 29 | 29 | "errors" | |
| 30 | 30 | "fmt" | |
| 31 | + | "slices" | |
| 31 | 32 | "strconv" | |
| 32 | 33 | "strings" | |
| 33 | 34 | "sync" | |
| @@ -358,14 +359,7 @@ func (d *Daemon) renameTab(sess *session, tb *tab, name string) error { | |||
| 358 | 359 | return errors.New("daemon: session closed") | |
| 359 | 360 | } | |
| 360 | 361 | sess.mu.Lock() | |
| 361 | - | found := false | |
| 362 | - | for _, candidate := range sess.tabs { | |
| 363 | - | if candidate == tb { | |
| 364 | - | found = true | |
| 365 | - | break | |
| 366 | - | } | |
| 367 | - | } | |
| 368 | - | if !found { | |
| 362 | + | if !slices.Contains(sess.tabs, tb) { | |
| 369 | 363 | sess.mu.Unlock() | |
| 370 | 364 | d.mu.Unlock() | |
| 371 | 365 | return errors.New("tab not found") | |
| @@ -410,6 +404,11 @@ func (s *session) persistRecordLocked(updatedAt int64) persist.Record { | |||
| 410 | 404 | } | |
| 411 | 405 | ||
| 412 | 406 | func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |
| 407 | + | d.mu.Lock() | |
| 408 | + | if d.sessions[sess.id] != sess { | |
| 409 | + | d.mu.Unlock() | |
| 410 | + | return | |
| 411 | + | } | |
| 413 | 412 | sess.mu.Lock() | |
| 414 | 413 | idx := -1 | |
| 415 | 414 | for i, w := range sess.tabs { | |
| @@ -420,10 +419,12 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||
| 420 | 419 | } | |
| 421 | 420 | if idx == -1 { | |
| 422 | 421 | sess.mu.Unlock() | |
| 422 | + | d.mu.Unlock() | |
| 423 | 423 | return | |
| 424 | 424 | } | |
| 425 | 425 | if len(sess.tabs) == 1 { | |
| 426 | 426 | sess.mu.Unlock() | |
| 427 | + | d.mu.Unlock() | |
| 427 | 428 | _ = d.killSession(sess, ports.ReasonSessionKilled, false) | |
| 428 | 429 | return | |
| 429 | 430 | } | |
| @@ -437,13 +438,14 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||
| 437 | 438 | ac := sess.client | |
| 438 | 439 | ephemeral := sess.ephemeral | |
| 439 | 440 | record := sess.persistRecordLocked(time.Now().UnixNano()) | |
| 440 | - | sess.mu.Unlock() | |
| 441 | - | ||
| 442 | 441 | if !ephemeral { | |
| 443 | 442 | if err := d.persist.Save(record); err != nil { | |
| 444 | 443 | d.log.Warn("persisting closed tab failed", "err", err, "session", record.Name) | |
| 445 | 444 | } | |
| 446 | 445 | } | |
| 446 | + | sess.mu.Unlock() | |
| 447 | + | d.mu.Unlock() | |
| 448 | + | ||
| 447 | 449 | d.clearDestroyedTabPreview(tb) | |
| 448 | 450 | if tb.cancel != nil { | |
| 449 | 451 | tb.cancel() | |
| @@ -28,6 +28,7 @@ import ( | |||||
| 28 | "context" | 28 | "context" | ||
| 29 | "errors" | 29 | "errors" | ||
| 30 | "fmt" | 30 | "fmt" | ||
| 31 | + | "slices" | |||
| 31 | "strconv" | 32 | "strconv" | ||
| 32 | "strings" | 33 | "strings" | ||
| 33 | "sync" | 34 | "sync" | ||
| @@ -358,14 +359,7 @@ func (d *Daemon) renameTab(sess *session, tb *tab, name string) error { | |||||
| 358 | return errors.New("daemon: session closed") | 359 | return errors.New("daemon: session closed") | ||
| 359 | } | 360 | } | ||
| 360 | sess.mu.Lock() | 361 | sess.mu.Lock() | ||
| 361 | - | found := false | 362 | + | if !slices.Contains(sess.tabs, tb) { |
| 362 | - | for _, candidate := range sess.tabs { | |||
| 363 | - | if candidate == tb { | |||
| 364 | - | found = true | |||
| 365 | - | break | |||
| 366 | - | } | |||
| 367 | - | } | |||
| 368 | - | if !found { | |||
| 369 | sess.mu.Unlock() | 363 | sess.mu.Unlock() | ||
| 370 | d.mu.Unlock() | 364 | d.mu.Unlock() | ||
| 371 | return errors.New("tab not found") | 365 | return errors.New("tab not found") | ||
| @@ -410,6 +404,11 @@ func (s *session) persistRecordLocked(updatedAt int64) persist.Record { | |||||
| 410 | } | 404 | } | ||
| 411 | 405 | ||||
| 412 | func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | 406 | func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | ||
| 407 | + | d.mu.Lock() | |||
| 408 | + | if d.sessions[sess.id] != sess { | |||
| 409 | + | d.mu.Unlock() | |||
| 410 | + | return | |||
| 411 | + | } | |||
| 413 | sess.mu.Lock() | 412 | sess.mu.Lock() | ||
| 414 | idx := -1 | 413 | idx := -1 | ||
| 415 | for i, w := range sess.tabs { | 414 | for i, w := range sess.tabs { | ||
| @@ -420,10 +419,12 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||||
| 420 | } | 419 | } | ||
| 421 | if idx == -1 { | 420 | if idx == -1 { | ||
| 422 | sess.mu.Unlock() | 421 | sess.mu.Unlock() | ||
| 422 | + | d.mu.Unlock() | |||
| 423 | return | 423 | return | ||
| 424 | } | 424 | } | ||
| 425 | if len(sess.tabs) == 1 { | 425 | if len(sess.tabs) == 1 { | ||
| 426 | sess.mu.Unlock() | 426 | sess.mu.Unlock() | ||
| 427 | + | d.mu.Unlock() | |||
| 427 | _ = d.killSession(sess, ports.ReasonSessionKilled, false) | 428 | _ = d.killSession(sess, ports.ReasonSessionKilled, false) | ||
| 428 | return | 429 | return | ||
| 429 | } | 430 | } | ||
| @@ -437,13 +438,14 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||||
| 437 | ac := sess.client | 438 | ac := sess.client | ||
| 438 | ephemeral := sess.ephemeral | 439 | ephemeral := sess.ephemeral | ||
| 439 | record := sess.persistRecordLocked(time.Now().UnixNano()) | 440 | record := sess.persistRecordLocked(time.Now().UnixNano()) | ||
| 440 | - | sess.mu.Unlock() | |||
| 441 | - | ||||
| 442 | if !ephemeral { | 441 | if !ephemeral { | ||
| 443 | if err := d.persist.Save(record); err != nil { | 442 | if err := d.persist.Save(record); err != nil { | ||
| 444 | d.log.Warn("persisting closed tab failed", "err", err, "session", record.Name) | 443 | d.log.Warn("persisting closed tab failed", "err", err, "session", record.Name) | ||
| 445 | } | 444 | } | ||
| 446 | } | 445 | } | ||
| 446 | + | sess.mu.Unlock() | |||
| 447 | + | d.mu.Unlock() | |||
| 448 | + | ||||
| 447 | d.clearDestroyedTabPreview(tb) | 449 | d.clearDestroyedTabPreview(tb) | ||
| 448 | if tb.cancel != nil { | 450 | if tb.cancel != nil { | ||
| 449 | tb.cancel() | 451 | tb.cancel() | ||
M internal/usecase/daemon/session_test.go +2 -2
| @@ -650,13 +650,13 @@ func (s *mockStoreState) has(name string) bool { | |||
| 650 | 650 | func TestDaemonLoadsPersistedSessionsAsStopped(t *testing.T) { | |
| 651 | 651 | store, _ := newMockStore(t) | |
| 652 | 652 | seed := New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithStore(store)) | |
| 653 | - | require.NoError(t, seed.persist.Save(persist.Record{Name: "work", Cwd: "/tmp/work", CreatedAt: 7, UpdatedAt: 8})) | |
| 653 | + | require.NoError(t, seed.persist.Save(persist.Record{Name: "work", Cwd: "/tmp/work", CreatedAt: 7, UpdatedAt: 8, TabNames: []string{"shell", "logs"}})) | |
| 654 | 654 | ||
| 655 | 655 | d := New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithStore(store)) | |
| 656 | 656 | d.mu.Lock() | |
| 657 | 657 | stopped := d.stopped["work"] | |
| 658 | 658 | d.mu.Unlock() | |
| 659 | - | require.Equal(t, stoppedSession{name: "work", cwd: "/tmp/work", createdAt: 7}, stopped) | |
| 659 | + | require.Equal(t, stoppedSession{name: "work", cwd: "/tmp/work", createdAt: 7, tabNames: []string{"shell", "logs"}}, stopped) | |
| 660 | 660 | } | |
| 661 | 661 | ||
| 662 | 662 | func TestCreateRenameKillPersistenceLifecycle(t *testing.T) { | |
| @@ -650,13 +650,13 @@ func (s *mockStoreState) has(name string) bool { | |||||
| 650 | func TestDaemonLoadsPersistedSessionsAsStopped(t *testing.T) { | 650 | func TestDaemonLoadsPersistedSessionsAsStopped(t *testing.T) { | ||
| 651 | store, _ := newMockStore(t) | 651 | store, _ := newMockStore(t) | ||
| 652 | seed := New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithStore(store)) | 652 | seed := New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithStore(store)) | ||
| 653 | - | require.NoError(t, seed.persist.Save(persist.Record{Name: "work", Cwd: "/tmp/work", CreatedAt: 7, UpdatedAt: 8})) | 653 | + | require.NoError(t, seed.persist.Save(persist.Record{Name: "work", Cwd: "/tmp/work", CreatedAt: 7, UpdatedAt: 8, TabNames: []string{"shell", "logs"}})) |
| 654 | 654 | ||||
| 655 | d := New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithStore(store)) | 655 | d := New(nil, stubClock{}, slog.New(slog.NewTextHandler(io.Discard, nil)), WithStore(store)) | ||
| 656 | d.mu.Lock() | 656 | d.mu.Lock() | ||
| 657 | stopped := d.stopped["work"] | 657 | stopped := d.stopped["work"] | ||
| 658 | d.mu.Unlock() | 658 | d.mu.Unlock() | ||
| 659 | - | require.Equal(t, stoppedSession{name: "work", cwd: "/tmp/work", createdAt: 7}, stopped) | 659 | + | require.Equal(t, stoppedSession{name: "work", cwd: "/tmp/work", createdAt: 7, tabNames: []string{"shell", "logs"}}, stopped) |
| 660 | } | 660 | } | ||
| 661 | 661 | ||||
| 662 | func TestCreateRenameKillPersistenceLifecycle(t *testing.T) { | 662 | func TestCreateRenameKillPersistenceLifecycle(t *testing.T) { | ||