detached 12d44e0

fix(daemon): make snapshot purge retryable

commit
12d44e014784f8d42889c13a3e0435d5ea583c51
author
brice · · 2026-07-21 23:23 UTC
parent
a9b46bf
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
11 files · +473 -69
Unified
UnifiedSplit
M internal/ports/mocks/store_mock.go +141 -0
@@ -801,3 +801,144 @@ func (_c *MockLegacySnapshotSource_LoadLegacy_Call) RunAndReturn(run func(contex
801801 _c.Call.Return(run)
802802 return _c
803803 }
804+
805+// NewMockSnapshotDeletionTombstone creates a new instance of MockSnapshotDeletionTombstone. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
806+// The first argument is typically a *testing.T value.
807+func NewMockSnapshotDeletionTombstone(t interface {
808+ mock.TestingT
809+ Cleanup(func())
810+}) *MockSnapshotDeletionTombstone {
811+ mock := &MockSnapshotDeletionTombstone{}
812+ mock.Mock.Test(t)
813+
814+ t.Cleanup(func() { mock.AssertExpectations(t) })
815+
816+ return mock
817+}
818+
819+// MockSnapshotDeletionTombstone is an autogenerated mock type for the SnapshotDeletionTombstone type
820+type MockSnapshotDeletionTombstone struct {
821+ mock.Mock
822+}
823+
824+type MockSnapshotDeletionTombstone_Expecter struct {
825+ mock *mock.Mock
826+}
827+
828+func (_m *MockSnapshotDeletionTombstone) EXPECT() *MockSnapshotDeletionTombstone_Expecter {
829+ return &MockSnapshotDeletionTombstone_Expecter{mock: &_m.Mock}
830+}
831+
832+// DeleteTombstone provides a mock function for the type MockSnapshotDeletionTombstone
833+func (_mock *MockSnapshotDeletionTombstone) DeleteTombstone(context1 context.Context, s string) error {
834+ ret := _mock.Called(context1, s)
835+
836+ if len(ret) == 0 {
837+ panic("no return value specified for DeleteTombstone")
838+ }
839+
840+ var r0 error
841+ if returnFunc, ok := ret.Get(0).(func(context.Context, string) error); ok {
842+ r0 = returnFunc(context1, s)
843+ } else {
844+ r0 = ret.Error(0)
845+ }
846+ return r0
847+}
848+
849+// MockSnapshotDeletionTombstone_DeleteTombstone_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteTombstone'
850+type MockSnapshotDeletionTombstone_DeleteTombstone_Call struct {
851+ *mock.Call
852+}
853+
854+// DeleteTombstone is a helper method to define mock.On call
855+// - context1 context.Context
856+// - s string
857+func (_e *MockSnapshotDeletionTombstone_Expecter) DeleteTombstone(context1 any, s any) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
858+ return &MockSnapshotDeletionTombstone_DeleteTombstone_Call{Call: _e.mock.On("DeleteTombstone", context1, s)}
859+}
860+
861+func (_c *MockSnapshotDeletionTombstone_DeleteTombstone_Call) Run(run func(context1 context.Context, s string)) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
862+ _c.Call.Run(func(args mock.Arguments) {
863+ var arg0 context.Context
864+ if args[0] != nil {
865+ arg0 = args[0].(context.Context)
866+ }
867+ var arg1 string
868+ if args[1] != nil {
869+ arg1 = args[1].(string)
870+ }
871+ run(
872+ arg0,
873+ arg1,
874+ )
875+ })
876+ return _c
877+}
878+
879+func (_c *MockSnapshotDeletionTombstone_DeleteTombstone_Call) Return(err error) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
880+ _c.Call.Return(err)
881+ return _c
882+}
883+
884+func (_c *MockSnapshotDeletionTombstone_DeleteTombstone_Call) RunAndReturn(run func(context1 context.Context, s string) error) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
885+ _c.Call.Return(run)
886+ return _c
887+}
888+
889+// Tombstone provides a mock function for the type MockSnapshotDeletionTombstone
890+func (_mock *MockSnapshotDeletionTombstone) Tombstone(context1 context.Context, s string) error {
891+ ret := _mock.Called(context1, s)
892+
893+ if len(ret) == 0 {
894+ panic("no return value specified for Tombstone")
895+ }
896+
897+ var r0 error
898+ if returnFunc, ok := ret.Get(0).(func(context.Context, string) error); ok {
899+ r0 = returnFunc(context1, s)
900+ } else {
901+ r0 = ret.Error(0)
902+ }
903+ return r0
904+}
905+
906+// MockSnapshotDeletionTombstone_Tombstone_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Tombstone'
907+type MockSnapshotDeletionTombstone_Tombstone_Call struct {
908+ *mock.Call
909+}
910+
911+// Tombstone is a helper method to define mock.On call
912+// - context1 context.Context
913+// - s string
914+func (_e *MockSnapshotDeletionTombstone_Expecter) Tombstone(context1 any, s any) *MockSnapshotDeletionTombstone_Tombstone_Call {
915+ return &MockSnapshotDeletionTombstone_Tombstone_Call{Call: _e.mock.On("Tombstone", context1, s)}
916+}
917+
918+func (_c *MockSnapshotDeletionTombstone_Tombstone_Call) Run(run func(context1 context.Context, s string)) *MockSnapshotDeletionTombstone_Tombstone_Call {
919+ _c.Call.Run(func(args mock.Arguments) {
920+ var arg0 context.Context
921+ if args[0] != nil {
922+ arg0 = args[0].(context.Context)
923+ }
924+ var arg1 string
925+ if args[1] != nil {
926+ arg1 = args[1].(string)
927+ }
928+ run(
929+ arg0,
930+ arg1,
931+ )
932+ })
933+ return _c
934+}
935+
936+func (_c *MockSnapshotDeletionTombstone_Tombstone_Call) Return(err error) *MockSnapshotDeletionTombstone_Tombstone_Call {
937+ _c.Call.Return(err)
938+ return _c
939+}
940+
941+func (_c *MockSnapshotDeletionTombstone_Tombstone_Call) RunAndReturn(run func(context1 context.Context, s string) error) *MockSnapshotDeletionTombstone_Tombstone_Call {
942+ _c.Call.Return(run)
943+ return _c
944+}
@@ -801,3 +801,144 @@ func (_c *MockLegacySnapshotSource_LoadLegacy_Call) RunAndReturn(run func(contex
801 _c.Call.Return(run)801 _c.Call.Return(run)
802 return _c802 return _c
803 }803 }
804+
805+// NewMockSnapshotDeletionTombstone creates a new instance of MockSnapshotDeletionTombstone. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations.
806+// The first argument is typically a *testing.T value.
807+func NewMockSnapshotDeletionTombstone(t interface {
808+ mock.TestingT
809+ Cleanup(func())
810+}) *MockSnapshotDeletionTombstone {
811+ mock := &MockSnapshotDeletionTombstone{}
812+ mock.Mock.Test(t)
813+
814+ t.Cleanup(func() { mock.AssertExpectations(t) })
815+
816+ return mock
817+}
818+
819+// MockSnapshotDeletionTombstone is an autogenerated mock type for the SnapshotDeletionTombstone type
820+type MockSnapshotDeletionTombstone struct {
821+ mock.Mock
822+}
823+
824+type MockSnapshotDeletionTombstone_Expecter struct {
825+ mock *mock.Mock
826+}
827+
828+func (_m *MockSnapshotDeletionTombstone) EXPECT() *MockSnapshotDeletionTombstone_Expecter {
829+ return &MockSnapshotDeletionTombstone_Expecter{mock: &_m.Mock}
830+}
831+
832+// DeleteTombstone provides a mock function for the type MockSnapshotDeletionTombstone
833+func (_mock *MockSnapshotDeletionTombstone) DeleteTombstone(context1 context.Context, s string) error {
834+ ret := _mock.Called(context1, s)
835+
836+ if len(ret) == 0 {
837+ panic("no return value specified for DeleteTombstone")
838+ }
839+
840+ var r0 error
841+ if returnFunc, ok := ret.Get(0).(func(context.Context, string) error); ok {
842+ r0 = returnFunc(context1, s)
843+ } else {
844+ r0 = ret.Error(0)
845+ }
846+ return r0
847+}
848+
849+// MockSnapshotDeletionTombstone_DeleteTombstone_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'DeleteTombstone'
850+type MockSnapshotDeletionTombstone_DeleteTombstone_Call struct {
851+ *mock.Call
852+}
853+
854+// DeleteTombstone is a helper method to define mock.On call
855+// - context1 context.Context
856+// - s string
857+func (_e *MockSnapshotDeletionTombstone_Expecter) DeleteTombstone(context1 any, s any) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
858+ return &MockSnapshotDeletionTombstone_DeleteTombstone_Call{Call: _e.mock.On("DeleteTombstone", context1, s)}
859+}
860+
861+func (_c *MockSnapshotDeletionTombstone_DeleteTombstone_Call) Run(run func(context1 context.Context, s string)) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
862+ _c.Call.Run(func(args mock.Arguments) {
863+ var arg0 context.Context
864+ if args[0] != nil {
865+ arg0 = args[0].(context.Context)
866+ }
867+ var arg1 string
868+ if args[1] != nil {
869+ arg1 = args[1].(string)
870+ }
871+ run(
872+ arg0,
873+ arg1,
874+ )
875+ })
876+ return _c
877+}
878+
879+func (_c *MockSnapshotDeletionTombstone_DeleteTombstone_Call) Return(err error) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
880+ _c.Call.Return(err)
881+ return _c
882+}
883+
884+func (_c *MockSnapshotDeletionTombstone_DeleteTombstone_Call) RunAndReturn(run func(context1 context.Context, s string) error) *MockSnapshotDeletionTombstone_DeleteTombstone_Call {
885+ _c.Call.Return(run)
886+ return _c
887+}
888+
889+// Tombstone provides a mock function for the type MockSnapshotDeletionTombstone
890+func (_mock *MockSnapshotDeletionTombstone) Tombstone(context1 context.Context, s string) error {
891+ ret := _mock.Called(context1, s)
892+
893+ if len(ret) == 0 {
894+ panic("no return value specified for Tombstone")
895+ }
896+
897+ var r0 error
898+ if returnFunc, ok := ret.Get(0).(func(context.Context, string) error); ok {
899+ r0 = returnFunc(context1, s)
900+ } else {
901+ r0 = ret.Error(0)
902+ }
903+ return r0
904+}
905+
906+// MockSnapshotDeletionTombstone_Tombstone_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Tombstone'
907+type MockSnapshotDeletionTombstone_Tombstone_Call struct {
908+ *mock.Call
909+}
910+
911+// Tombstone is a helper method to define mock.On call
912+// - context1 context.Context
913+// - s string
914+func (_e *MockSnapshotDeletionTombstone_Expecter) Tombstone(context1 any, s any) *MockSnapshotDeletionTombstone_Tombstone_Call {
915+ return &MockSnapshotDeletionTombstone_Tombstone_Call{Call: _e.mock.On("Tombstone", context1, s)}
916+}
917+
918+func (_c *MockSnapshotDeletionTombstone_Tombstone_Call) Run(run func(context1 context.Context, s string)) *MockSnapshotDeletionTombstone_Tombstone_Call {
919+ _c.Call.Run(func(args mock.Arguments) {
920+ var arg0 context.Context
921+ if args[0] != nil {
922+ arg0 = args[0].(context.Context)
923+ }
924+ var arg1 string
925+ if args[1] != nil {
926+ arg1 = args[1].(string)
927+ }
928+ run(
929+ arg0,
930+ arg1,
931+ )
932+ })
933+ return _c
934+}
935+
936+func (_c *MockSnapshotDeletionTombstone_Tombstone_Call) Return(err error) *MockSnapshotDeletionTombstone_Tombstone_Call {
937+ _c.Call.Return(err)
938+ return _c
939+}
940+
941+func (_c *MockSnapshotDeletionTombstone_Tombstone_Call) RunAndReturn(run func(context1 context.Context, s string) error) *MockSnapshotDeletionTombstone_Tombstone_Call {
942+ _c.Call.Return(run)
943+ return _c
944+}
M internal/ports/store.go +9 -0
@@ -73,3 +73,12 @@ type LegacySnapshotSource interface {
7373 LoadLegacy(context.Context) ([]LegacySnapshot, error)
7474 DeleteLegacy(context.Context, string) error
7575 }
76+
77+// SnapshotDeletionTombstone makes a named-session purge durable across the
78+// independent incremental and legacy snapshot sources. A tombstoned name must
79+// not be restored or imported until both source deletions and metadata removal
80+// have completed.
81+type SnapshotDeletionTombstone interface {
82+ Tombstone(context.Context, string) error
83+ DeleteTombstone(context.Context, string) error
84+}
@@ -73,3 +73,12 @@ type LegacySnapshotSource interface {
73 LoadLegacy(context.Context) ([]LegacySnapshot, error)73 LoadLegacy(context.Context) ([]LegacySnapshot, error)
74 DeleteLegacy(context.Context, string) error74 DeleteLegacy(context.Context, string) error
75 }75 }
76+
77+// SnapshotDeletionTombstone makes a named-session purge durable across the
78+// independent incremental and legacy snapshot sources. A tombstoned name must
79+// not be restored or imported until both source deletions and metadata removal
80+// have completed.
81+type SnapshotDeletionTombstone interface {
82+ Tombstone(context.Context, string) error
83+ DeleteTombstone(context.Context, string) error
84+}
M internal/usecase/daemon/daemon.go +12 -3
@@ -121,9 +121,13 @@ type Daemon struct {
121121 // read-only migration input and is never used for new writes.
122122 snapshotRepository ports.SnapshotRepository
123123 legacySnapshots ports.LegacySnapshotSource
124- snapsEnabled bool
125- noticeStore ports.NoticeStore
126- snapshotMarshal func(snapcodec.Session) ([]byte, error)
124+ // snapshotDeletion durably fences a live purge across both snapshot
125+ // sources. Production repositories must provide it; nil preserves the
126+ // narrower checkpoint-only test seam.
127+ snapshotDeletion ports.SnapshotDeletionTombstone
128+ snapsEnabled bool
129+ noticeStore ports.NoticeStore
130+ snapshotMarshal func(snapcodec.Session) ([]byte, error)
127131 // snapshotChunkCache contains only encoded immutable sealed chunks. It is
128132 // independent of pane state and is bounded to prevent checkpoint history
129133 // from becoming unbounded daemon memory.
@@ -259,6 +263,11 @@ func WithSnapshotRepository(repository ports.SnapshotRepository, legacy ports.Le
259263 return func(d *Daemon) {
260264 d.snapshotRepository = repository
261265 d.legacySnapshots = legacy
266+ if deletion, ok := repository.(ports.SnapshotDeletionTombstone); ok {
267+ d.snapshotDeletion = deletion
268+ } else if deletion, ok := legacy.(ports.SnapshotDeletionTombstone); ok {
269+ d.snapshotDeletion = deletion
270+ }
262271 d.snapsEnabled = repository != nil
263272 }
264273 }
@@ -121,9 +121,13 @@ type Daemon struct {
121 // read-only migration input and is never used for new writes.121 // read-only migration input and is never used for new writes.
122 snapshotRepository ports.SnapshotRepository122 snapshotRepository ports.SnapshotRepository
123 legacySnapshots ports.LegacySnapshotSource123 legacySnapshots ports.LegacySnapshotSource
124- snapsEnabled bool124+ // snapshotDeletion durably fences a live purge across both snapshot
125- noticeStore ports.NoticeStore125+ // sources. Production repositories must provide it; nil preserves the
126- snapshotMarshal func(snapcodec.Session) ([]byte, error)126+ // narrower checkpoint-only test seam.
127+ snapshotDeletion ports.SnapshotDeletionTombstone
128+ snapsEnabled bool
129+ noticeStore ports.NoticeStore
130+ snapshotMarshal func(snapcodec.Session) ([]byte, error)
127 // snapshotChunkCache contains only encoded immutable sealed chunks. It is131 // snapshotChunkCache contains only encoded immutable sealed chunks. It is
128 // independent of pane state and is bounded to prevent checkpoint history132 // independent of pane state and is bounded to prevent checkpoint history
129 // from becoming unbounded daemon memory.133 // from becoming unbounded daemon memory.
@@ -259,6 +263,11 @@ func WithSnapshotRepository(repository ports.SnapshotRepository, legacy ports.Le
259 return func(d *Daemon) {263 return func(d *Daemon) {
260 d.snapshotRepository = repository264 d.snapshotRepository = repository
261 d.legacySnapshots = legacy265 d.legacySnapshots = legacy
266+ if deletion, ok := repository.(ports.SnapshotDeletionTombstone); ok {
267+ d.snapshotDeletion = deletion
268+ } else if deletion, ok := legacy.(ports.SnapshotDeletionTombstone); ok {
269+ d.snapshotDeletion = deletion
270+ }
262 d.snapsEnabled = repository != nil271 d.snapsEnabled = repository != nil
263 }272 }
264 }273 }
M internal/usecase/daemon/picker.go +3 -10
@@ -547,17 +547,10 @@ func (d *Daemon) resumeStoppedAndSwitch(from *session, ac *attachedClient, targe
547547
548548 func (d *Daemon) killPickerTarget(target picker.Target) error {
549549 if target.Stopped {
550- d.mu.Lock()
551- stopped, ok := d.stopped[target.Name]
552- if ok && !stopped.purging {
553- if err := d.persist.Delete(target.Name); err != nil {
554- d.mu.Unlock()
555- d.log.Warn("deleting persisted stopped session failed", "err", err, "session", target.Name)
556- return domain.UserErr(domain.NoticePersistDelete, "couldn't delete stopped session", err)
557- }
558- delete(d.stopped, target.Name)
550+ if err := d.retryStoppedPurge(target.Name); err != nil {
551+ d.log.Warn("deleting persisted stopped session failed", "err", err, "session", target.Name)
552+ return domain.UserErr(domain.NoticePersistDelete, "couldn't delete stopped session", err)
559553 }
560- d.mu.Unlock()
561554 return nil
562555 }
563556 d.mu.Lock()
@@ -547,17 +547,10 @@ func (d *Daemon) resumeStoppedAndSwitch(from *session, ac *attachedClient, targe
547 547
548 func (d *Daemon) killPickerTarget(target picker.Target) error {548 func (d *Daemon) killPickerTarget(target picker.Target) error {
549 if target.Stopped {549 if target.Stopped {
550- d.mu.Lock()550+ if err := d.retryStoppedPurge(target.Name); err != nil {
551- stopped, ok := d.stopped[target.Name]551+ d.log.Warn("deleting persisted stopped session failed", "err", err, "session", target.Name)
552- if ok && !stopped.purging {552+ return domain.UserErr(domain.NoticePersistDelete, "couldn't delete stopped session", err)
553- if err := d.persist.Delete(target.Name); err != nil {
554- d.mu.Unlock()
555- d.log.Warn("deleting persisted stopped session failed", "err", err, "session", target.Name)
556- return domain.UserErr(domain.NoticePersistDelete, "couldn't delete stopped session", err)
557- }
558- delete(d.stopped, target.Name)
559 }553 }
560- d.mu.Unlock()
561 return nil554 return nil
562 }555 }
563 d.mu.Lock()556 d.mu.Lock()
M internal/usecase/daemon/session.go +103 -30
@@ -790,21 +790,104 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) {
790790 // ptyReader drains child output into the VT screen and pokes the dirty channel
791791 // (non-blocking: a full channel already means a render is pending). On any read
792792
793+// beginSnapshotPurge commits a logical named-session kill before its live
794+// identity or persisted metadata is removed. It never runs under daemon or
795+// session locks.
796+func (d *Daemon) beginSnapshotPurge(name string) error {
797+ if d.snapshotDeletion == nil || name == "" {
798+ return nil
799+ }
800+ return d.snapshotDeletion.Tombstone(context.Background(), name)
801+}
802+
803+// finishSnapshotPurge deletes both independently durable snapshot sources,
804+// then metadata, and only then clears the tombstone. Any failure deliberately
805+// leaves the marker in place so startup cannot restore or import the name and
806+// a later live/offline kill can retry the idempotent source deletes.
807+func (d *Daemon) finishSnapshotPurge(name string) error {
808+ var incrementalErr, legacyErr error
809+ if d.snapshotRepository != nil {
810+ incrementalErr = d.snapshotRepository.Delete(context.Background(), name)
811+ }
812+ if d.legacySnapshots != nil {
813+ legacyErr = d.legacySnapshots.DeleteLegacy(context.Background(), name)
814+ }
815+ if incrementalErr != nil || legacyErr != nil {
816+ return errors.Join(incrementalErr, legacyErr)
817+ }
818+ if err := d.persist.Delete(name); err != nil {
819+ return err
820+ }
821+ if d.snapshotDeletion != nil {
822+ return d.snapshotDeletion.DeleteTombstone(context.Background(), name)
823+ }
824+ return nil
825+}
826+
827+// retryStoppedPurge completes a previously closed session's durable purge.
828+// It keeps the stopped record hidden until all sources and metadata cross their
829+// deletion boundaries.
830+func (d *Daemon) retryStoppedPurge(name string) error {
831+ d.mu.Lock()
832+ stopped, ok := d.stopped[name]
833+ if !ok {
834+ d.mu.Unlock()
835+ return nil
836+ }
837+ // A stopped session without a durable snapshot source retains the existing
838+ // metadata-only delete behavior. There is no restore/import source to fence.
839+ if d.snapshotDeletion == nil {
840+ d.mu.Unlock()
841+ if err := d.persist.Delete(name); err != nil {
842+ return err
843+ }
844+ d.mu.Lock()
845+ if _, ok := d.stopped[name]; ok {
846+ delete(d.stopped, name)
847+ }
848+ d.mu.Unlock()
849+ return nil
850+ }
851+ stopped.purging = true
852+ d.stopped[name] = stopped
853+ d.mu.Unlock()
854+
855+ if err := d.beginSnapshotPurge(name); err != nil {
856+ return err
857+ }
858+ if err := d.finishSnapshotPurge(name); err != nil {
859+ return err
860+ }
861+ d.mu.Lock()
862+ if current, ok := d.stopped[name]; ok && current.purging {
863+ delete(d.stopped, name)
864+ }
865+ d.mu.Unlock()
866+ return nil
867+}
868+
793869 func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error {
794870 ringing := sess.anyAttention()
795871 sess.mu.Lock()
796872 isEphemeral := sess.ephemeral
873+ name := sess.name
797874 sess.mu.Unlock()
798- // A repository delete must never race an older publication that could
799- // recreate the just-quarantined name. Do not hold daemon or session locks
800- // while joining: a repository implementation may ignore cancellation.
801- if purge && !isEphemeral && d.snapshotRepository != nil {
802- <-quarantineSnapshotCoordinator(sess)
875+ // Commit the tombstone before changing live identity or metadata. A
876+ // repository delete must never race an older publication that could recreate
877+ // the just-quarantined name. Do not hold daemon or session locks while
878+ // tombstoning or joining: a repository implementation may block on I/O.
879+ if purge && !isEphemeral {
880+ if err := d.beginSnapshotPurge(name); err != nil {
881+ return err
882+ }
883+ if d.snapshotRepository != nil {
884+ <-quarantineSnapshotCoordinator(sess)
885+ }
803886 }
804887 if !purge && !isEphemeral && d.persistEnabled {
805888 d.refreshSessionCwd(sess)
806889 }
807- var snapshotDeleteErr, terminalSnapshotErr error
890+ var terminalSnapshotErr error
808891 if d.snapsEnabled && !isEphemeral {
809892 if purge {
810893 if d.snapshotRepository != nil {
@@ -858,25 +941,7 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error {
858941 d.mu.Unlock()
859942 d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)
860943
861- purgeErr := snapshotDeleteErr
862- if !ephemeral && purge {
863- if err := d.persist.Delete(stoppedName); err != nil {
864- purgeErr = err
865- d.log.Warn("deleting persisted session failed", "err", err, "session", stoppedName)
866- d.mu.Lock()
867- if stopped, ok := d.stopped[stoppedName]; ok && stopped.purging {
868- stopped.purging = false
869- d.stopped[stoppedName] = stopped
870- }
871- d.mu.Unlock()
872- } else {
873- d.mu.Lock()
874- if stopped, ok := d.stopped[stoppedName]; ok && stopped.purging {
875- delete(d.stopped, stoppedName)
876- }
877- d.mu.Unlock()
878- }
879- }
944+ var purgeErr error
880945
881946 // A global route that selected this attachment must publish before terminal
882947 // teardown makes it stale.
@@ -920,12 +985,20 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error {
920985 d.log.Warn("removing clipboard temp file failed", "err", err, "path", path)
921986 }
922987 }
923- // The coordinator and all panes are now stopped, so no producer can
924- // publish another generation after this destructive repository delete.
925- if !ephemeral && purge && d.snapshotRepository != nil {
926- if err := d.snapshotRepository.Delete(context.Background(), stoppedName); err != nil {
988+ // The coordinator and all panes are now stopped, so no producer can publish
989+ // another generation after this destructive source deletion. Keep the
990+ // tombstone and hidden stopped record when any source or metadata operation
991+ // fails; retryStoppedPurge (including a repeated live kill) resumes safely.
992+ if !ephemeral && purge {
993+ if err := d.finishSnapshotPurge(stoppedName); err != nil {
927994 purgeErr = errors.Join(purgeErr, err)
928- d.log.Warn("deleting session snapshot failed", "err", err, "session", stoppedName)
995+ d.log.Warn("finishing session snapshot purge failed", "err", err, "session", stoppedName)
996+ } else {
997+ d.mu.Lock()
998+ if stopped, ok := d.stopped[stoppedName]; ok && stopped.purging {
999+ delete(d.stopped, stoppedName)
1000+ }
1001+ d.mu.Unlock()
9291002 }
9301003 }
9311004 if empty {
@@ -790,21 +790,104 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) {
790 // ptyReader drains child output into the VT screen and pokes the dirty channel790 // ptyReader drains child output into the VT screen and pokes the dirty channel
791 // (non-blocking: a full channel already means a render is pending). On any read791 // (non-blocking: a full channel already means a render is pending). On any read
792 792
793+// beginSnapshotPurge commits a logical named-session kill before its live
794+// identity or persisted metadata is removed. It never runs under daemon or
795+// session locks.
796+func (d *Daemon) beginSnapshotPurge(name string) error {
797+ if d.snapshotDeletion == nil || name == "" {
798+ return nil
799+ }
800+ return d.snapshotDeletion.Tombstone(context.Background(), name)
801+}
802+
803+// finishSnapshotPurge deletes both independently durable snapshot sources,
804+// then metadata, and only then clears the tombstone. Any failure deliberately
805+// leaves the marker in place so startup cannot restore or import the name and
806+// a later live/offline kill can retry the idempotent source deletes.
807+func (d *Daemon) finishSnapshotPurge(name string) error {
808+ var incrementalErr, legacyErr error
809+ if d.snapshotRepository != nil {
810+ incrementalErr = d.snapshotRepository.Delete(context.Background(), name)
811+ }
812+ if d.legacySnapshots != nil {
813+ legacyErr = d.legacySnapshots.DeleteLegacy(context.Background(), name)
814+ }
815+ if incrementalErr != nil || legacyErr != nil {
816+ return errors.Join(incrementalErr, legacyErr)
817+ }
818+ if err := d.persist.Delete(name); err != nil {
819+ return err
820+ }
821+ if d.snapshotDeletion != nil {
822+ return d.snapshotDeletion.DeleteTombstone(context.Background(), name)
823+ }
824+ return nil
825+}
826+
827+// retryStoppedPurge completes a previously closed session's durable purge.
828+// It keeps the stopped record hidden until all sources and metadata cross their
829+// deletion boundaries.
830+func (d *Daemon) retryStoppedPurge(name string) error {
831+ d.mu.Lock()
832+ stopped, ok := d.stopped[name]
833+ if !ok {
834+ d.mu.Unlock()
835+ return nil
836+ }
837+ // A stopped session without a durable snapshot source retains the existing
838+ // metadata-only delete behavior. There is no restore/import source to fence.
839+ if d.snapshotDeletion == nil {
840+ d.mu.Unlock()
841+ if err := d.persist.Delete(name); err != nil {
842+ return err
843+ }
844+ d.mu.Lock()
845+ if _, ok := d.stopped[name]; ok {
846+ delete(d.stopped, name)
847+ }
848+ d.mu.Unlock()
849+ return nil
850+ }
851+ stopped.purging = true
852+ d.stopped[name] = stopped
853+ d.mu.Unlock()
854+
855+ if err := d.beginSnapshotPurge(name); err != nil {
856+ return err
857+ }
858+ if err := d.finishSnapshotPurge(name); err != nil {
859+ return err
860+ }
861+ d.mu.Lock()
862+ if current, ok := d.stopped[name]; ok && current.purging {
863+ delete(d.stopped, name)
864+ }
865+ d.mu.Unlock()
866+ return nil
867+}
868+
793 func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error {869 func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error {
794 ringing := sess.anyAttention()870 ringing := sess.anyAttention()
795 sess.mu.Lock()871 sess.mu.Lock()
796 isEphemeral := sess.ephemeral872 isEphemeral := sess.ephemeral
873+ name := sess.name
797 sess.mu.Unlock()874 sess.mu.Unlock()
798- // A repository delete must never race an older publication that could875+ // Commit the tombstone before changing live identity or metadata. A
799- // recreate the just-quarantined name. Do not hold daemon or session locks876+ // repository delete must never race an older publication that could recreate
800- // while joining: a repository implementation may ignore cancellation.877+ // the just-quarantined name. Do not hold daemon or session locks while
801- if purge && !isEphemeral && d.snapshotRepository != nil {878+ // tombstoning or joining: a repository implementation may block on I/O.
802- <-quarantineSnapshotCoordinator(sess)879+ if purge && !isEphemeral {
880+ if err := d.beginSnapshotPurge(name); err != nil {
881+ return err
882+ }
883+ if d.snapshotRepository != nil {
884+ <-quarantineSnapshotCoordinator(sess)
885+ }
803 }886 }
804 if !purge && !isEphemeral && d.persistEnabled {887 if !purge && !isEphemeral && d.persistEnabled {
805 d.refreshSessionCwd(sess)888 d.refreshSessionCwd(sess)
806 }889 }
807- var snapshotDeleteErr, terminalSnapshotErr error890+ var terminalSnapshotErr error
808 if d.snapsEnabled && !isEphemeral {891 if d.snapsEnabled && !isEphemeral {
809 if purge {892 if purge {
810 if d.snapshotRepository != nil {893 if d.snapshotRepository != nil {
@@ -858,25 +941,7 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error {
858 d.mu.Unlock()941 d.mu.Unlock()
859 d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)942 d.log.Info("session closed", "session", stoppedName, "id", sess.id, "ephemeral", ephemeral, "purge", purge, "reason", reason)
860 943
861- purgeErr := snapshotDeleteErr944+ var purgeErr error
862- if !ephemeral && purge {
863- if err := d.persist.Delete(stoppedName); err != nil {
864- purgeErr = err
865- d.log.Warn("deleting persisted session failed", "err", err, "session", stoppedName)
866- d.mu.Lock()
867- if stopped, ok := d.stopped[stoppedName]; ok && stopped.purging {
868- stopped.purging = false
869- d.stopped[stoppedName] = stopped
870- }
871- d.mu.Unlock()
872- } else {
873- d.mu.Lock()
874- if stopped, ok := d.stopped[stoppedName]; ok && stopped.purging {
875- delete(d.stopped, stoppedName)
876- }
877- d.mu.Unlock()
878- }
879- }
880 945
881 // A global route that selected this attachment must publish before terminal946 // A global route that selected this attachment must publish before terminal
882 // teardown makes it stale.947 // teardown makes it stale.
@@ -920,12 +985,20 @@ func (d *Daemon) killSession(sess *session, reason uint8, purge bool) error {
920 d.log.Warn("removing clipboard temp file failed", "err", err, "path", path)985 d.log.Warn("removing clipboard temp file failed", "err", err, "path", path)
921 }986 }
922 }987 }
923- // The coordinator and all panes are now stopped, so no producer can988+ // The coordinator and all panes are now stopped, so no producer can publish
924- // publish another generation after this destructive repository delete.989+ // another generation after this destructive source deletion. Keep the
925- if !ephemeral && purge && d.snapshotRepository != nil {990+ // tombstone and hidden stopped record when any source or metadata operation
926- if err := d.snapshotRepository.Delete(context.Background(), stoppedName); err != nil {991+ // fails; retryStoppedPurge (including a repeated live kill) resumes safely.
992+ if !ephemeral && purge {
993+ if err := d.finishSnapshotPurge(stoppedName); err != nil {
927 purgeErr = errors.Join(purgeErr, err)994 purgeErr = errors.Join(purgeErr, err)
928- d.log.Warn("deleting session snapshot failed", "err", err, "session", stoppedName)995+ d.log.Warn("finishing session snapshot purge failed", "err", err, "session", stoppedName)
996+ } else {
997+ d.mu.Lock()
998+ if stopped, ok := d.stopped[stoppedName]; ok && stopped.purging {
999+ delete(d.stopped, stoppedName)
1000+ }
1001+ d.mu.Unlock()
929 }1002 }
930 }1003 }
931 if empty {1004 if empty {
A internal/usecase/daemon/session_purge_retry_test.go +92 -0
@@ -0,0 +1,92 @@
1+package daemon
2+
3+import (
4+ "context"
5+ "errors"
6+ "testing"
7+
8+ "github.com/stretchr/testify/require"
9+
10+ "github.com/bnema/vev/internal/domain"
11+ "github.com/bnema/vev/internal/ports"
12+ portsmocks "github.com/bnema/vev/internal/ports/mocks"
13+)
14+
15+type retryablePurgeRepository struct {
16+ incrementalErr error
17+ legacyErr error
18+ tombstoned map[string]bool
19+ calls []string
20+}
21+
22+func (r *retryablePurgeRepository) Publish(context.Context, ports.SnapshotPublication) error {
23+ return nil
24+}
25+func (r *retryablePurgeRepository) List(context.Context) ([]string, error) { return nil, nil }
26+func (r *retryablePurgeRepository) Load(context.Context, string) (ports.SnapshotGeneration, error) {
27+ return ports.SnapshotGeneration{}, errors.New("unused")
28+}
29+func (r *retryablePurgeRepository) Maintain(context.Context) error { return nil }
30+func (r *retryablePurgeRepository) Delete(_ context.Context, name string) error {
31+ r.calls = append(r.calls, "incremental")
32+ return r.incrementalErr
33+}
34+func (r *retryablePurgeRepository) LoadLegacy(context.Context) ([]ports.LegacySnapshot, error) {
35+ return nil, nil
36+}
37+func (r *retryablePurgeRepository) DeleteLegacy(_ context.Context, name string) error {
38+ r.calls = append(r.calls, "legacy")
39+ return r.legacyErr
40+}
41+func (r *retryablePurgeRepository) Tombstone(_ context.Context, name string) error {
42+ if r.tombstoned == nil {
43+ r.tombstoned = make(map[string]bool)
44+ }
45+ r.calls = append(r.calls, "tombstone")
46+ r.tombstoned[name] = true
47+ return nil
48+}
49+func (r *retryablePurgeRepository) DeleteTombstone(_ context.Context, name string) error {
50+ r.calls = append(r.calls, "clear tombstone")
51+ delete(r.tombstoned, name)
52+ return nil
53+}
54+
55+func TestLivePurgeRetainsTombstoneAcrossPartialSourceDeletion(t *testing.T) {
56+ for _, tt := range []struct {
57+ name string
58+ incremental error
59+ legacy error
60+ }{
61+ {name: "incremental failure", incremental: errors.New("incremental failed")},
62+ {name: "legacy failure", legacy: errors.New("legacy failed")},
63+ } {
64+ t.Run(tt.name, func(t *testing.T) {
65+ d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
66+ repository := &retryablePurgeRepository{incrementalErr: tt.incremental, legacyErr: tt.legacy}
67+ WithSnapshotRepository(repository, repository)(d)
68+ sess := newSnapshotTestSession(t, "work", false, "/work")
69+ sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY).EXPECT().Close().Return(nil).Once()
70+ d.sessions = map[domain.SessionID]*session{sess.id: sess}
71+
72+ require.Error(t, d.killSession(sess, ports.ReasonSessionKilled, true))
73+ require.True(t, repository.tombstoned["work"], "partial purge must fence startup restore/import")
74+ require.Equal(t, []string{"tombstone", "incremental", "legacy"}, repository.calls)
75+ d.mu.Lock()
76+ stopped, ok := d.stopped["work"]
77+ d.mu.Unlock()
78+ require.True(t, ok)
79+ require.True(t, stopped.purging)
80+
81+ repository.incrementalErr = nil
82+ repository.legacyErr = nil
83+ require.NoError(t, d.retryStoppedPurge("work"))
84+ require.False(t, repository.tombstoned["work"])
85+ require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "clear tombstone"}, repository.calls)
86+ d.mu.Lock()
87+ _, ok = d.stopped["work"]
88+ d.mu.Unlock()
89+ require.False(t, ok, "metadata is removed only after both source deletes and tombstone clear")
90+ })
91+ }
92+}
@@ -0,0 +1,92 @@
1+package daemon
2+
3+import (
4+ "context"
5+ "errors"
6+ "testing"
7+
8+ "github.com/stretchr/testify/require"
9+
10+ "github.com/bnema/vev/internal/domain"
11+ "github.com/bnema/vev/internal/ports"
12+ portsmocks "github.com/bnema/vev/internal/ports/mocks"
13+)
14+
15+type retryablePurgeRepository struct {
16+ incrementalErr error
17+ legacyErr error
18+ tombstoned map[string]bool
19+ calls []string
20+}
21+
22+func (r *retryablePurgeRepository) Publish(context.Context, ports.SnapshotPublication) error {
23+ return nil
24+}
25+func (r *retryablePurgeRepository) List(context.Context) ([]string, error) { return nil, nil }
26+func (r *retryablePurgeRepository) Load(context.Context, string) (ports.SnapshotGeneration, error) {
27+ return ports.SnapshotGeneration{}, errors.New("unused")
28+}
29+func (r *retryablePurgeRepository) Maintain(context.Context) error { return nil }
30+func (r *retryablePurgeRepository) Delete(_ context.Context, name string) error {
31+ r.calls = append(r.calls, "incremental")
32+ return r.incrementalErr
33+}
34+func (r *retryablePurgeRepository) LoadLegacy(context.Context) ([]ports.LegacySnapshot, error) {
35+ return nil, nil
36+}
37+func (r *retryablePurgeRepository) DeleteLegacy(_ context.Context, name string) error {
38+ r.calls = append(r.calls, "legacy")
39+ return r.legacyErr
40+}
41+func (r *retryablePurgeRepository) Tombstone(_ context.Context, name string) error {
42+ if r.tombstoned == nil {
43+ r.tombstoned = make(map[string]bool)
44+ }
45+ r.calls = append(r.calls, "tombstone")
46+ r.tombstoned[name] = true
47+ return nil
48+}
49+func (r *retryablePurgeRepository) DeleteTombstone(_ context.Context, name string) error {
50+ r.calls = append(r.calls, "clear tombstone")
51+ delete(r.tombstoned, name)
52+ return nil
53+}
54+
55+func TestLivePurgeRetainsTombstoneAcrossPartialSourceDeletion(t *testing.T) {
56+ for _, tt := range []struct {
57+ name string
58+ incremental error
59+ legacy error
60+ }{
61+ {name: "incremental failure", incremental: errors.New("incremental failed")},
62+ {name: "legacy failure", legacy: errors.New("legacy failed")},
63+ } {
64+ t.Run(tt.name, func(t *testing.T) {
65+ d := newTestDaemon(t, portsmocks.NewMockPTYFactory(t), stubClock{})
66+ repository := &retryablePurgeRepository{incrementalErr: tt.incremental, legacyErr: tt.legacy}
67+ WithSnapshotRepository(repository, repository)(d)
68+ sess := newSnapshotTestSession(t, "work", false, "/work")
69+ sess.tabs[0].panes["pane-1"].pty.(*portsmocks.MockPTY).EXPECT().Close().Return(nil).Once()
70+ d.sessions = map[domain.SessionID]*session{sess.id: sess}
71+
72+ require.Error(t, d.killSession(sess, ports.ReasonSessionKilled, true))
73+ require.True(t, repository.tombstoned["work"], "partial purge must fence startup restore/import")
74+ require.Equal(t, []string{"tombstone", "incremental", "legacy"}, repository.calls)
75+ d.mu.Lock()
76+ stopped, ok := d.stopped["work"]
77+ d.mu.Unlock()
78+ require.True(t, ok)
79+ require.True(t, stopped.purging)
80+
81+ repository.incrementalErr = nil
82+ repository.legacyErr = nil
83+ require.NoError(t, d.retryStoppedPurge("work"))
84+ require.False(t, repository.tombstoned["work"])
85+ require.Equal(t, []string{"tombstone", "incremental", "legacy", "tombstone", "incremental", "legacy", "clear tombstone"}, repository.calls)
86+ d.mu.Lock()
87+ _, ok = d.stopped["work"]
88+ d.mu.Unlock()
89+ require.False(t, ok, "metadata is removed only after both source deletes and tombstone clear")
90+ })
91+ }
92+}
M internal/usecase/daemon/snapshot.go +14 -14
@@ -63,12 +63,13 @@ type snapshotCapturePane struct {
6363 // history is retained for the v3 compatibility encoder. Incremental
6464 // captures keep sealed identities and a copied tail separately so capture
6565 // never rotates a live mutable tail merely to persist it.
66- history vt.HistoryView
67- sealed vt.HistorySnapshotView
68- tail vt.HistoryView
69- visible []byte
70- visibleErr error
71- process *snapcodec.Process
66+ history vt.HistoryView
67+ sealed vt.HistorySnapshotView
68+ tail vt.HistoryView
69+ // visible is an owned copy taken while pane.mu is held. It must be
70+ // marshaled only after that lock has been released.
71+ visible vt.PrimaryVisibleSnapshot
72+ process *snapcodec.Process
7273 }
7374
7475 // snapshotFailureSignature classifies a persistence failure without retaining
@@ -686,12 +687,10 @@ func (d *Daemon) captureSnapshotState(sess *session, generation uint64) (*snapsh
686687 if pty != nil {
687688 pid = pty.Pid()
688689 }
689- visible, visibleErr := p.screen.MarshalPrimaryVisible()
690690 paneCapture := snapshotCapturePane{
691- id: p.id,
692- stableID: p.stableID,
693- visible: visible,
694- visibleErr: visibleErr,
691+ id: p.id,
692+ stableID: p.stableID,
693+ visible: p.screen.PrimaryVisibleSnapshot(),
695694 }
696695 paneCapture.sealed = p.history.SnapshotView()
697696 paneCapture.tail = paneCapture.sealed.Tail()
@@ -731,10 +730,11 @@ func (d *Daemon) encodeSnapshotCapture(capture *snapshotCapture) ([]byte, error)
731730 if historyErr != nil {
732731 return nil, fmt.Errorf("terminal snapshot pane %s history: %w", paneCapture.id, historyErr)
733732 }
734- if paneCapture.visibleErr != nil {
735- return nil, fmt.Errorf("terminal snapshot pane %s visible: %w", paneCapture.id, paneCapture.visibleErr)
733+ visible, visibleErr := paneCapture.visible.Marshal()
734+ if visibleErr != nil {
735+ return nil, fmt.Errorf("terminal snapshot pane %s visible: %w", paneCapture.id, visibleErr)
736736 }
737- tabSnap.Panes = append(tabSnap.Panes, snapcodec.Pane{ID: paneCapture.id, StableID: paneCapture.stableID, Cwd: paneCapture.cwd, SealedChunks: sealed, Tail: tail, Visible: paneCapture.visible, Process: paneCapture.process})
737+ tabSnap.Panes = append(tabSnap.Panes, snapcodec.Pane{ID: paneCapture.id, StableID: paneCapture.stableID, Cwd: paneCapture.cwd, SealedChunks: sealed, Tail: tail, Visible: visible, Process: paneCapture.process})
738738 paneIDs[paneCapture.id] = struct{}{}
739739 }
740740 if tabSnap.Tree != nil {
@@ -63,12 +63,13 @@ type snapshotCapturePane struct {
63 // history is retained for the v3 compatibility encoder. Incremental63 // history is retained for the v3 compatibility encoder. Incremental
64 // captures keep sealed identities and a copied tail separately so capture64 // captures keep sealed identities and a copied tail separately so capture
65 // never rotates a live mutable tail merely to persist it.65 // never rotates a live mutable tail merely to persist it.
66- history vt.HistoryView66+ history vt.HistoryView
67- sealed vt.HistorySnapshotView67+ sealed vt.HistorySnapshotView
68- tail vt.HistoryView68+ tail vt.HistoryView
69- visible []byte69+ // visible is an owned copy taken while pane.mu is held. It must be
70- visibleErr error70+ // marshaled only after that lock has been released.
71- process *snapcodec.Process71+ visible vt.PrimaryVisibleSnapshot
72+ process *snapcodec.Process
72 }73 }
73 74
74 // snapshotFailureSignature classifies a persistence failure without retaining75 // snapshotFailureSignature classifies a persistence failure without retaining
@@ -686,12 +687,10 @@ func (d *Daemon) captureSnapshotState(sess *session, generation uint64) (*snapsh
686 if pty != nil {687 if pty != nil {
687 pid = pty.Pid()688 pid = pty.Pid()
688 }689 }
689- visible, visibleErr := p.screen.MarshalPrimaryVisible()
690 paneCapture := snapshotCapturePane{690 paneCapture := snapshotCapturePane{
691- id: p.id,691+ id: p.id,
692- stableID: p.stableID,692+ stableID: p.stableID,
693- visible: visible,693+ visible: p.screen.PrimaryVisibleSnapshot(),
694- visibleErr: visibleErr,
695 }694 }
696 paneCapture.sealed = p.history.SnapshotView()695 paneCapture.sealed = p.history.SnapshotView()
697 paneCapture.tail = paneCapture.sealed.Tail()696 paneCapture.tail = paneCapture.sealed.Tail()
@@ -731,10 +730,11 @@ func (d *Daemon) encodeSnapshotCapture(capture *snapshotCapture) ([]byte, error)
731 if historyErr != nil {730 if historyErr != nil {
732 return nil, fmt.Errorf("terminal snapshot pane %s history: %w", paneCapture.id, historyErr)731 return nil, fmt.Errorf("terminal snapshot pane %s history: %w", paneCapture.id, historyErr)
733 }732 }
734- if paneCapture.visibleErr != nil {733+ visible, visibleErr := paneCapture.visible.Marshal()
735- return nil, fmt.Errorf("terminal snapshot pane %s visible: %w", paneCapture.id, paneCapture.visibleErr)734+ if visibleErr != nil {
735+ return nil, fmt.Errorf("terminal snapshot pane %s visible: %w", paneCapture.id, visibleErr)
736 }736 }
737- tabSnap.Panes = append(tabSnap.Panes, snapcodec.Pane{ID: paneCapture.id, StableID: paneCapture.stableID, Cwd: paneCapture.cwd, SealedChunks: sealed, Tail: tail, Visible: paneCapture.visible, Process: paneCapture.process})737+ tabSnap.Panes = append(tabSnap.Panes, snapcodec.Pane{ID: paneCapture.id, StableID: paneCapture.stableID, Cwd: paneCapture.cwd, SealedChunks: sealed, Tail: tail, Visible: visible, Process: paneCapture.process})
738 paneIDs[paneCapture.id] = struct{}{}738 paneIDs[paneCapture.id] = struct{}{}
739 }739 }
740 if tabSnap.Tree != nil {740 if tabSnap.Tree != nil {
M internal/usecase/daemon/snapshot_incremental.go +6 -3
@@ -101,8 +101,11 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho
101101 for _, tab := range capture.tabs {
102102 outTab := snapcodec.ManifestTab{StableID: tab.stableID, Cols: tab.cols, Rows: tab.rows, NextPaneID: tab.nextPaneID, Focus: tab.focus, Tree: tab.tree, Panes: make([]snapcodec.ManifestPane, 0, len(tab.panes))}
103103 for _, pane := range tab.panes {
104- if pane.visibleErr != nil {
105- return ports.SnapshotPublication{}, fmt.Errorf("snapshot visible: %w", pane.visibleErr)
104+ // visible was copied under pane.mu. Marshal it here in the worker,
105+ // after all pane and session locks have been released.
106+ visible, err := pane.visible.Marshal()
107+ if err != nil {
108+ return ports.SnapshotPublication{}, fmt.Errorf("snapshot visible: %w", err)
106109 }
107110 outPane := snapcodec.ManifestPane{ID: pane.id, StableID: pane.stableID, Cwd: pane.cwd, Process: pane.process}
108111 sealedCount := pane.sealed.ChunkCount()
@@ -130,7 +133,7 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho
130133 if err != nil {
131134 return ports.SnapshotPublication{}, err
132135 }
133- visibleObject, err := snapcodec.MarshalObject(snapcodec.Visible, pane.visible)
136+ visibleObject, err := snapcodec.MarshalObject(snapcodec.Visible, visible)
134137 if err != nil {
135138 return ports.SnapshotPublication{}, err
136139 }
@@ -101,8 +101,11 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho
101 for _, tab := range capture.tabs {101 for _, tab := range capture.tabs {
102 outTab := snapcodec.ManifestTab{StableID: tab.stableID, Cols: tab.cols, Rows: tab.rows, NextPaneID: tab.nextPaneID, Focus: tab.focus, Tree: tab.tree, Panes: make([]snapcodec.ManifestPane, 0, len(tab.panes))}102 outTab := snapcodec.ManifestTab{StableID: tab.stableID, Cols: tab.cols, Rows: tab.rows, NextPaneID: tab.nextPaneID, Focus: tab.focus, Tree: tab.tree, Panes: make([]snapcodec.ManifestPane, 0, len(tab.panes))}
103 for _, pane := range tab.panes {103 for _, pane := range tab.panes {
104- if pane.visibleErr != nil {104+ // visible was copied under pane.mu. Marshal it here in the worker,
105- return ports.SnapshotPublication{}, fmt.Errorf("snapshot visible: %w", pane.visibleErr)105+ // after all pane and session locks have been released.
106+ visible, err := pane.visible.Marshal()
107+ if err != nil {
108+ return ports.SnapshotPublication{}, fmt.Errorf("snapshot visible: %w", err)
106 }109 }
107 outPane := snapcodec.ManifestPane{ID: pane.id, StableID: pane.stableID, Cwd: pane.cwd, Process: pane.process}110 outPane := snapcodec.ManifestPane{ID: pane.id, StableID: pane.stableID, Cwd: pane.cwd, Process: pane.process}
108 sealedCount := pane.sealed.ChunkCount()111 sealedCount := pane.sealed.ChunkCount()
@@ -130,7 +133,7 @@ func (d *Daemon) incrementalPublication(capture *snapshotCapture) (ports.Snapsho
130 if err != nil {133 if err != nil {
131 return ports.SnapshotPublication{}, err134 return ports.SnapshotPublication{}, err
132 }135 }
133- visibleObject, err := snapcodec.MarshalObject(snapcodec.Visible, pane.visible)136+ visibleObject, err := snapcodec.MarshalObject(snapcodec.Visible, visible)
134 if err != nil {137 if err != nil {
135 return ports.SnapshotPublication{}, err138 return ports.SnapshotPublication{}, err
136 }139 }
M internal/usecase/daemon/snapshot_incremental_test.go +29 -4
@@ -2,19 +2,44 @@ package daemon
22
33 import (
44 "testing"
5+ "time"
56
67 "github.com/bnema/vev/pkg/renderer"
78 "github.com/bnema/vev/pkg/vt"
89 )
910
11+func TestIncrementalPublicationEncodesAfterPaneUnlock(t *testing.T) {
12+ d := New(nil, nil, nil)
13+ sess := newSnapshotTestSession(t, "work", false, "/work")
14+ capture, ok := d.captureSnapshotState(sess, 1)
15+ if !ok {
16+ t.Fatal("capture rejected named session")
17+ }
18+
19+ pane := sess.tabs[0].panes["pane-1"]
20+ pane.mu.Lock()
21+ done := make(chan error, 1)
22+ go func() {
23+ _, err := d.incrementalPublication(capture)
24+ done <- err
25+ }()
26+ select {
27+ case err := <-done:
28+ if err != nil {
29+ t.Fatalf("encode capture: %v", err)
30+ }
31+ case <-time.After(testWaitTimeout):
32+ pane.mu.Unlock()
33+ t.Fatal("visible encoding waited for pane lock")
34+ }
35+ pane.mu.Unlock()
36+}
37+
1038 func TestIncrementalPublicationReusesSealedChunkObject(t *testing.T) {
1139 history := vt.NewHistory(vt.HistoryConfig{MaxRows: 2, ChunkRows: 1})
1240 history.Append([]renderer.Cell{renderer.BlankCell()})
1341 view := history.SealAndView()
14- visible, err := vt.MarshalVisible(renderer.NewFrame(1, 1))
15- if err != nil {
16- t.Fatal(err)
17- }
42+ visible := vt.NewScreen(1, 1).PrimaryVisibleSnapshot()
1843 d := New(nil, nil, nil)
1944 capture := &snapshotCapture{name: "work", generation: 1, tabs: []snapshotCaptureTab{{stableID: "t", cols: 1, rows: 1, panes: []snapshotCapturePane{{id: "p", stableID: "p", history: view, visible: visible}}}}}
2045 first, err := d.incrementalPublication(capture)
@@ -2,19 +2,44 @@ package daemon
2 2
3 import (3 import (
4 "testing"4 "testing"
5+ "time"
5 6
6 "github.com/bnema/vev/pkg/renderer"7 "github.com/bnema/vev/pkg/renderer"
7 "github.com/bnema/vev/pkg/vt"8 "github.com/bnema/vev/pkg/vt"
8 )9 )
9 10
11+func TestIncrementalPublicationEncodesAfterPaneUnlock(t *testing.T) {
12+ d := New(nil, nil, nil)
13+ sess := newSnapshotTestSession(t, "work", false, "/work")
14+ capture, ok := d.captureSnapshotState(sess, 1)
15+ if !ok {
16+ t.Fatal("capture rejected named session")
17+ }
18+
19+ pane := sess.tabs[0].panes["pane-1"]
20+ pane.mu.Lock()
21+ done := make(chan error, 1)
22+ go func() {
23+ _, err := d.incrementalPublication(capture)
24+ done <- err
25+ }()
26+ select {
27+ case err := <-done:
28+ if err != nil {
29+ t.Fatalf("encode capture: %v", err)
30+ }
31+ case <-time.After(testWaitTimeout):
32+ pane.mu.Unlock()
33+ t.Fatal("visible encoding waited for pane lock")
34+ }
35+ pane.mu.Unlock()
36+}
37+
10 func TestIncrementalPublicationReusesSealedChunkObject(t *testing.T) {38 func TestIncrementalPublicationReusesSealedChunkObject(t *testing.T) {
11 history := vt.NewHistory(vt.HistoryConfig{MaxRows: 2, ChunkRows: 1})39 history := vt.NewHistory(vt.HistoryConfig{MaxRows: 2, ChunkRows: 1})
12 history.Append([]renderer.Cell{renderer.BlankCell()})40 history.Append([]renderer.Cell{renderer.BlankCell()})
13 view := history.SealAndView()41 view := history.SealAndView()
14- visible, err := vt.MarshalVisible(renderer.NewFrame(1, 1))42+ visible := vt.NewScreen(1, 1).PrimaryVisibleSnapshot()
15- if err != nil {
16- t.Fatal(err)
17- }
18 d := New(nil, nil, nil)43 d := New(nil, nil, nil)
19 capture := &snapshotCapture{name: "work", generation: 1, tabs: []snapshotCaptureTab{{stableID: "t", cols: 1, rows: 1, panes: []snapshotCapturePane{{id: "p", stableID: "p", history: view, visible: visible}}}}}44 capture := &snapshotCapture{name: "work", generation: 1, tabs: []snapshotCaptureTab{{stableID: "t", cols: 1, rows: 1, panes: []snapshotCapturePane{{id: "p", stableID: "p", history: view, visible: visible}}}}}
20 first, err := d.incrementalPublication(capture)45 first, err := d.incrementalPublication(capture)
A pkg/vt/primary_visible_snapshot_test.go +29 -0
@@ -0,0 +1,29 @@
1+package vt
2+
3+import (
4+ "testing"
5+
6+ "github.com/stretchr/testify/require"
7+)
8+
9+func TestPrimaryVisibleSnapshotOwnsSavedPrimaryAcrossLiveMutations(t *testing.T) {
10+ s := NewScreen(8, 2)
11+ s.Write([]byte("primary"))
12+ s.Write([]byte("\x1b[?1049h"))
13+
14+ captured := s.PrimaryVisibleSnapshot()
15+ mutated := make(chan struct{})
16+ go func() {
17+ s.Write([]byte("alternate"))
18+ s.Resize(3, 3)
19+ s.Write([]byte("\x1b[?1049lmutated"))
20+ close(mutated)
21+ }()
22+
23+ blob, err := captured.Marshal()
24+ <-mutated
25+ require.NoError(t, err)
26+ restored := NewScreen(1, 1)
27+ require.NoError(t, restored.RestorePrimaryVisible(blob))
28+ require.Equal(t, "primary ", rowString(restored.Frame.Row(0)))
29+}
@@ -0,0 +1,29 @@
1+package vt
2+
3+import (
4+ "testing"
5+
6+ "github.com/stretchr/testify/require"
7+)
8+
9+func TestPrimaryVisibleSnapshotOwnsSavedPrimaryAcrossLiveMutations(t *testing.T) {
10+ s := NewScreen(8, 2)
11+ s.Write([]byte("primary"))
12+ s.Write([]byte("\x1b[?1049h"))
13+
14+ captured := s.PrimaryVisibleSnapshot()
15+ mutated := make(chan struct{})
16+ go func() {
17+ s.Write([]byte("alternate"))
18+ s.Resize(3, 3)
19+ s.Write([]byte("\x1b[?1049lmutated"))
20+ close(mutated)
21+ }()
22+
23+ blob, err := captured.Marshal()
24+ <-mutated
25+ require.NoError(t, err)
26+ restored := NewScreen(1, 1)
27+ require.NoError(t, restored.RestorePrimaryVisible(blob))
28+ require.Equal(t, "primary ", rowString(restored.Frame.Row(0)))
29+}
M pkg/vt/snapshot.go +35 -5
@@ -141,17 +141,47 @@ func MarshalVisible(frame renderer.Frame) ([]byte, error) {
141141 return marshalVisible(frame, boundaries)
142142 }
143143
144-// MarshalPrimaryVisible preserves the primary buffer's logical-line boundaries
145-// alongside its cells so a restored viewport can subsequently reflow.
146-func (s *Screen) MarshalPrimaryVisible() ([]byte, error) {
144+// PrimaryVisibleSnapshot is an owned copy of the primary buffer's visible
145+// state. Capture it while the screen owner is synchronized, then Marshal it
146+// after releasing that lock.
147+type PrimaryVisibleSnapshot struct {
148+ frame renderer.Frame
149+ boundaries []lineBoundary
150+}
151+
152+// PrimaryVisibleSnapshot copies the saved primary frame and its logical-line
153+// boundaries. When the alternate screen is active, the saved primary buffer is
154+// copied rather than the currently displayed alternate buffer.
155+func (s *Screen) PrimaryVisibleSnapshot() PrimaryVisibleSnapshot {
147156 b := s.buffer
148157 if s.alternate != nil {
149158 b = s.alternate.buffer
150159 }
151160 if b == nil {
152- return MarshalVisible(s.PrimaryVisibleFrame())
161+ return PrimaryVisibleSnapshot{frame: s.PrimaryVisibleFrame()}
162+ }
163+ return PrimaryVisibleSnapshot{
164+ frame: b.frame.Clone(),
165+ boundaries: append([]lineBoundary(nil), b.boundaries...),
166+ }
167+}
168+
169+// Marshal encodes an owned primary visible snapshot without accessing live VT
170+// state.
171+func (v PrimaryVisibleSnapshot) Marshal() ([]byte, error) {
172+ if len(v.boundaries) == 0 && v.frame.Height > 0 {
173+ return MarshalVisible(v.frame)
153174 }
154- return marshalVisible(b.frame, b.boundaries)
175+ return marshalVisible(v.frame, v.boundaries)
176+}
177+
178+// MarshalPrimaryVisible preserves the primary buffer's logical-line boundaries
179+// alongside its cells so a restored viewport can subsequently reflow.
180+//
181+// Deprecated: callers holding a pane lock should use PrimaryVisibleSnapshot
182+// and marshal the result after unlocking.
183+func (s *Screen) MarshalPrimaryVisible() ([]byte, error) {
184+ return s.PrimaryVisibleSnapshot().Marshal()
155185 }
156186
157187 func marshalVisible(frame renderer.Frame, boundaries []lineBoundary) ([]byte, error) {
@@ -141,17 +141,47 @@ func MarshalVisible(frame renderer.Frame) ([]byte, error) {
141 return marshalVisible(frame, boundaries)141 return marshalVisible(frame, boundaries)
142 }142 }
143 143
144-// MarshalPrimaryVisible preserves the primary buffer's logical-line boundaries144+// PrimaryVisibleSnapshot is an owned copy of the primary buffer's visible
145-// alongside its cells so a restored viewport can subsequently reflow.145+// state. Capture it while the screen owner is synchronized, then Marshal it
146-func (s *Screen) MarshalPrimaryVisible() ([]byte, error) {146+// after releasing that lock.
147+type PrimaryVisibleSnapshot struct {
148+ frame renderer.Frame
149+ boundaries []lineBoundary
150+}
151+
152+// PrimaryVisibleSnapshot copies the saved primary frame and its logical-line
153+// boundaries. When the alternate screen is active, the saved primary buffer is
154+// copied rather than the currently displayed alternate buffer.
155+func (s *Screen) PrimaryVisibleSnapshot() PrimaryVisibleSnapshot {
147 b := s.buffer156 b := s.buffer
148 if s.alternate != nil {157 if s.alternate != nil {
149 b = s.alternate.buffer158 b = s.alternate.buffer
150 }159 }
151 if b == nil {160 if b == nil {
152- return MarshalVisible(s.PrimaryVisibleFrame())161+ return PrimaryVisibleSnapshot{frame: s.PrimaryVisibleFrame()}
162+ }
163+ return PrimaryVisibleSnapshot{
164+ frame: b.frame.Clone(),
165+ boundaries: append([]lineBoundary(nil), b.boundaries...),
166+ }
167+}
168+
169+// Marshal encodes an owned primary visible snapshot without accessing live VT
170+// state.
171+func (v PrimaryVisibleSnapshot) Marshal() ([]byte, error) {
172+ if len(v.boundaries) == 0 && v.frame.Height > 0 {
173+ return MarshalVisible(v.frame)
153 }174 }
154- return marshalVisible(b.frame, b.boundaries)175+ return marshalVisible(v.frame, v.boundaries)
176+}
177+
178+// MarshalPrimaryVisible preserves the primary buffer's logical-line boundaries
179+// alongside its cells so a restored viewport can subsequently reflow.
180+//
181+// Deprecated: callers holding a pane lock should use PrimaryVisibleSnapshot
182+// and marshal the result after unlocking.
183+func (s *Screen) MarshalPrimaryVisible() ([]byte, error) {
184+ return s.PrimaryVisibleSnapshot().Marshal()
155 }185 }
156 186
157 func marshalVisible(frame renderer.Frame, boundaries []lineBoundary) ([]byte, error) {187 func marshalVisible(frame renderer.Frame, boundaries []lineBoundary) ([]byte, error) {

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 12

Tags 11

↑↓ moveenter openesc close