fix(daemon): assign lifecycle on session promotion
- commit
- 5033bbea13b75aab2eb4974f833c0f4bef4df869
- author
- brice · · 2026-07-15 08:00 UTC
- parent
- 68453c3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +117 -4
M internal/usecase/daemon/palette.go +1 -3
| @@ -86,9 +86,7 @@ func (d *Daemon) paletteResults(current *session, commands []command.Command) [] | |||
| 86 | 86 | return left < right | |
| 87 | 87 | }) | |
| 88 | 88 | sort.Slice(stopped, func(i, j int) bool { return stopped[i].name < stopped[j].name }) | |
| 89 | - | for _, candidate := range active { | |
| 90 | - | results = append(results, candidate) | |
| 91 | - | } | |
| 89 | + | results = append(results, active...) | |
| 92 | 90 | for _, candidate := range stopped { | |
| 93 | 91 | results = append(results, palette.NewStoppedSessionResult(candidate.name, time.Unix(0, candidate.createdAt))) | |
| 94 | 92 | } | |
| @@ -86,9 +86,7 @@ func (d *Daemon) paletteResults(current *session, commands []command.Command) [] | |||||
| 86 | return left < right | 86 | return left < right | ||
| 87 | }) | 87 | }) | ||
| 88 | sort.Slice(stopped, func(i, j int) bool { return stopped[i].name < stopped[j].name }) | 88 | sort.Slice(stopped, func(i, j int) bool { return stopped[i].name < stopped[j].name }) | ||
| 89 | - | for _, candidate := range active { | 89 | + | results = append(results, active...) |
| 90 | - | results = append(results, candidate) | |||
| 91 | - | } | |||
| 92 | for _, candidate := range stopped { | 90 | for _, candidate := range stopped { | ||
| 93 | results = append(results, palette.NewStoppedSessionResult(candidate.name, time.Unix(0, candidate.createdAt))) | 91 | results = append(results, palette.NewStoppedSessionResult(candidate.name, time.Unix(0, candidate.createdAt))) | ||
| 94 | } | 92 | } | ||
M internal/usecase/daemon/session.go +10 -1
| @@ -511,9 +511,17 @@ func (d *Daemon) renameSession(sess *session, name string) error { | |||
| 511 | 511 | oldName := sess.name | |
| 512 | 512 | wasEphemeral := sess.ephemeral | |
| 513 | 513 | createdAt := sess.createdAt | |
| 514 | + | if wasEphemeral { | |
| 515 | + | var err error | |
| 516 | + | createdAt, err = d.allocateLifecycleCreatedAtLocked() | |
| 517 | + | if err != nil { | |
| 518 | + | sess.mu.Unlock() | |
| 519 | + | return err | |
| 520 | + | } | |
| 521 | + | } | |
| 514 | 522 | lastUsedSeq := sess.mruAt.Load() | |
| 515 | 523 | if wasEphemeral || oldName != name { | |
| 516 | - | record := sess.persistRecordLocked(time.Now().UnixNano()) | |
| 524 | + | record := sess.persistRecordLocked(d.nowUnixNano()) | |
| 517 | 525 | record.Name = name | |
| 518 | 526 | record.CreatedAt = createdAt | |
| 519 | 527 | record.LastUsedSeq = lastUsedSeq | |
| @@ -543,6 +551,7 @@ func (d *Daemon) renameSession(sess *session, name string) error { | |||
| 543 | 551 | delete(d.stopped, oldName) | |
| 544 | 552 | delete(d.stopped, name) | |
| 545 | 553 | sess.name = name | |
| 554 | + | sess.createdAt = createdAt | |
| 546 | 555 | sess.ephemeral = false | |
| 547 | 556 | sess.snapEligible.Store(name != "") | |
| 548 | 557 | sess.mu.Unlock() | |
| @@ -511,9 +511,17 @@ func (d *Daemon) renameSession(sess *session, name string) error { | |||||
| 511 | oldName := sess.name | 511 | oldName := sess.name | ||
| 512 | wasEphemeral := sess.ephemeral | 512 | wasEphemeral := sess.ephemeral | ||
| 513 | createdAt := sess.createdAt | 513 | createdAt := sess.createdAt | ||
| 514 | + | if wasEphemeral { | |||
| 515 | + | var err error | |||
| 516 | + | createdAt, err = d.allocateLifecycleCreatedAtLocked() | |||
| 517 | + | if err != nil { | |||
| 518 | + | sess.mu.Unlock() | |||
| 519 | + | return err | |||
| 520 | + | } | |||
| 521 | + | } | |||
| 514 | lastUsedSeq := sess.mruAt.Load() | 522 | lastUsedSeq := sess.mruAt.Load() | ||
| 515 | if wasEphemeral || oldName != name { | 523 | if wasEphemeral || oldName != name { | ||
| 516 | - | record := sess.persistRecordLocked(time.Now().UnixNano()) | 524 | + | record := sess.persistRecordLocked(d.nowUnixNano()) |
| 517 | record.Name = name | 525 | record.Name = name | ||
| 518 | record.CreatedAt = createdAt | 526 | record.CreatedAt = createdAt | ||
| 519 | record.LastUsedSeq = lastUsedSeq | 527 | record.LastUsedSeq = lastUsedSeq | ||
| @@ -543,6 +551,7 @@ func (d *Daemon) renameSession(sess *session, name string) error { | |||||
| 543 | delete(d.stopped, oldName) | 551 | delete(d.stopped, oldName) | ||
| 544 | delete(d.stopped, name) | 552 | delete(d.stopped, name) | ||
| 545 | sess.name = name | 553 | sess.name = name | ||
| 554 | + | sess.createdAt = createdAt | |||
| 546 | sess.ephemeral = false | 555 | sess.ephemeral = false | ||
| 547 | sess.snapEligible.Store(name != "") | 556 | sess.snapEligible.Store(name != "") | ||
| 548 | sess.mu.Unlock() | 557 | sess.mu.Unlock() | ||
M internal/usecase/daemon/session_test.go +106 -0
| @@ -2,6 +2,7 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | + | "errors" | |
| 5 | 6 | "io" | |
| 6 | 7 | "log/slog" | |
| 7 | 8 | "math" | |
| @@ -1030,6 +1031,111 @@ func TestEphemeralRenamePromotesAndStoppedCollisionRejected(t *testing.T) { | |||
| 1030 | 1031 | require.True(t, state.has("named")) | |
| 1031 | 1032 | } | |
| 1032 | 1033 | ||
| 1034 | + | func TestEphemeralPromotionAssignsAndPersistsLifecycleIdentity(t *testing.T) { | |
| 1035 | + | p, release := newBlockingPTY(t) | |
| 1036 | + | defer release() | |
| 1037 | + | store, state := newMockStore(t) | |
| 1038 | + | clock := &lifecycleClock{nows: []time.Time{time.Unix(0, 100)}} | |
| 1039 | + | d := newTestDaemon(t, newFactory(t, p), clock) | |
| 1040 | + | WithStore(store)(d) | |
| 1041 | + | ||
| 1042 | + | sess, err := d.createSessionLocked("0", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, d.baseEnv) | |
| 1043 | + | require.NoError(t, err) | |
| 1044 | + | require.Zero(t, sess.createdAt, "regression fixture: ephemerals have no lifecycle identity") | |
| 1045 | + | ||
| 1046 | + | require.NoError(t, d.renameSession(sess, "named")) | |
| 1047 | + | ||
| 1048 | + | require.False(t, sess.ephemeral) | |
| 1049 | + | require.Equal(t, "named", sess.name) | |
| 1050 | + | require.Equal(t, int64(100), sess.createdAt) | |
| 1051 | + | require.Equal(t, sess.createdAt, state.record(t, "named").CreatedAt) | |
| 1052 | + | } | |
| 1053 | + | ||
| 1054 | + | func TestEphemeralPromotionLifecyclePreventsStaleSameNamePaletteTarget(t *testing.T) { | |
| 1055 | + | fromPTY, releaseFrom := newBlockingPTY(t) | |
| 1056 | + | firstPTY, releaseFirst := newBlockingPTY(t) | |
| 1057 | + | secondPTY, releaseSecond := newBlockingPTY(t) | |
| 1058 | + | defer releaseFrom() | |
| 1059 | + | defer releaseFirst() | |
| 1060 | + | defer releaseSecond() | |
| 1061 | + | ||
| 1062 | + | d, from, ac, _ := newManualSessionWithPTYs(t, fromPTY) | |
| 1063 | + | store, _ := newMockStore(t) | |
| 1064 | + | clock := &lifecycleClock{nows: []time.Time{time.Unix(0, 100), time.Unix(0, 100)}} | |
| 1065 | + | d.clock = clock | |
| 1066 | + | d.ptys = newFactorySeq(t, firstPTY, secondPTY) | |
| 1067 | + | WithStore(store)(d) | |
| 1068 | + | ||
| 1069 | + | first, err := d.createSessionLocked("0", true, "/tmp", ac.size, terminalEnv{}, d.baseEnv) | |
| 1070 | + | require.NoError(t, err) | |
| 1071 | + | require.NoError(t, d.renameSession(first, "named")) | |
| 1072 | + | staleCreatedAt := first.createdAt | |
| 1073 | + | require.NotZero(t, staleCreatedAt) | |
| 1074 | + | require.NoError(t, d.killSession(first, ports.ReasonSessionKilled, true)) | |
| 1075 | + | d.sessWg.Wait() | |
| 1076 | + | ||
| 1077 | + | second, err := d.createSessionLocked("0", true, "/tmp", ac.size, terminalEnv{}, d.baseEnv) | |
| 1078 | + | require.NoError(t, err) | |
| 1079 | + | require.NoError(t, d.renameSession(second, "named")) | |
| 1080 | + | require.NotEqual(t, staleCreatedAt, second.createdAt) | |
| 1081 | + | ||
| 1082 | + | require.False(t, d.switchToTarget(from, ac, picker.Target{Name: "named", TabIndex: -1, ExpectedCreatedAt: &staleCreatedAt})) | |
| 1083 | + | require.Same(t, from, ac.currentSession()) | |
| 1084 | + | releaseSecond() | |
| 1085 | + | d.sessWg.Wait() | |
| 1086 | + | } | |
| 1087 | + | ||
| 1088 | + | func TestEphemeralPromotionLifecycleFailuresLeaveStateRollbackSafe(t *testing.T) { | |
| 1089 | + | t.Run("allocator exhaustion", func(t *testing.T) { | |
| 1090 | + | p, release := newBlockingPTY(t) | |
| 1091 | + | defer release() | |
| 1092 | + | store, state := newMockStore(t) | |
| 1093 | + | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |
| 1094 | + | WithStore(store)(d) | |
| 1095 | + | d.lastAllocatedCreatedAt = math.MaxInt64 | |
| 1096 | + | ||
| 1097 | + | sess, err := d.createSessionLocked("0", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, d.baseEnv) | |
| 1098 | + | require.NoError(t, err) | |
| 1099 | + | require.EqualError(t, d.renameSession(sess, "named"), "daemon: lifecycle identities exhausted") | |
| 1100 | + | require.Equal(t, "0", sess.name) | |
| 1101 | + | require.True(t, sess.ephemeral) | |
| 1102 | + | require.Zero(t, sess.createdAt) | |
| 1103 | + | require.Equal(t, int64(math.MaxInt64), d.lastAllocatedCreatedAt) | |
| 1104 | + | require.False(t, state.has("named")) | |
| 1105 | + | }) | |
| 1106 | + | ||
| 1107 | + | t.Run("persistence failure", func(t *testing.T) { | |
| 1108 | + | p, release := newBlockingPTY(t) | |
| 1109 | + | defer release() | |
| 1110 | + | store := portsmocks.NewMockStore(t) | |
| 1111 | + | var attempted map[string][]byte | |
| 1112 | + | store.EXPECT().Set(mock.Anything, mock.Anything).RunAndReturn(func(key, value []byte) error { | |
| 1113 | + | attempted = map[string][]byte{string(key): append([]byte(nil), value...)} | |
| 1114 | + | return errors.New("disk full") | |
| 1115 | + | }).Once() | |
| 1116 | + | clock := &lifecycleClock{nows: []time.Time{time.Unix(0, 100), time.Unix(0, 100)}} | |
| 1117 | + | d := newTestDaemon(t, newFactory(t, p), clock) | |
| 1118 | + | WithStore(store)(d) | |
| 1119 | + | ||
| 1120 | + | sess, err := d.createSessionLocked("0", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, d.baseEnv) | |
| 1121 | + | require.NoError(t, err) | |
| 1122 | + | require.EqualError(t, d.renameSession(sess, "named"), "disk full") | |
| 1123 | + | require.Equal(t, "0", sess.name) | |
| 1124 | + | require.True(t, sess.ephemeral) | |
| 1125 | + | require.Zero(t, sess.createdAt) | |
| 1126 | + | require.Equal(t, int64(100), d.lastAllocatedCreatedAt, "a failed durable write may be ambiguous, so its lifecycle identity remains reserved") | |
| 1127 | + | records, err := persist.New(newStaticStore(attempted)).LoadAll() | |
| 1128 | + | require.NoError(t, err) | |
| 1129 | + | require.Len(t, records, 1) | |
| 1130 | + | require.Equal(t, int64(100), records[0].CreatedAt, "a promotion must never attempt to persist a zero lifecycle identity") | |
| 1131 | + | ||
| 1132 | + | store.EXPECT().Set(mock.Anything, mock.Anything).Return(nil).Once() | |
| 1133 | + | store.EXPECT().Sync().Return(nil).Once() | |
| 1134 | + | require.NoError(t, d.renameSession(sess, "named")) | |
| 1135 | + | require.Equal(t, int64(101), sess.createdAt, "the high-water mark must remain monotonic after a failed promotion") | |
| 1136 | + | }) | |
| 1137 | + | } | |
| 1138 | + | ||
| 1033 | 1139 | func TestRefreshSessionCwdTouchesOnlyOnChange(t *testing.T) { | |
| 1034 | 1140 | sz := domain.Size{Cols: 80, Rows: 24} | |
| 1035 | 1141 | p, release := newBlockingPTY(t) | |
| @@ -2,6 +2,7 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | + | "errors" | |||
| 5 | "io" | 6 | "io" | ||
| 6 | "log/slog" | 7 | "log/slog" | ||
| 7 | "math" | 8 | "math" | ||
| @@ -1030,6 +1031,111 @@ func TestEphemeralRenamePromotesAndStoppedCollisionRejected(t *testing.T) { | |||||
| 1030 | require.True(t, state.has("named")) | 1031 | require.True(t, state.has("named")) | ||
| 1031 | } | 1032 | } | ||
| 1032 | 1033 | ||||
| 1034 | + | func TestEphemeralPromotionAssignsAndPersistsLifecycleIdentity(t *testing.T) { | |||
| 1035 | + | p, release := newBlockingPTY(t) | |||
| 1036 | + | defer release() | |||
| 1037 | + | store, state := newMockStore(t) | |||
| 1038 | + | clock := &lifecycleClock{nows: []time.Time{time.Unix(0, 100)}} | |||
| 1039 | + | d := newTestDaemon(t, newFactory(t, p), clock) | |||
| 1040 | + | WithStore(store)(d) | |||
| 1041 | + | ||||
| 1042 | + | sess, err := d.createSessionLocked("0", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, d.baseEnv) | |||
| 1043 | + | require.NoError(t, err) | |||
| 1044 | + | require.Zero(t, sess.createdAt, "regression fixture: ephemerals have no lifecycle identity") | |||
| 1045 | + | ||||
| 1046 | + | require.NoError(t, d.renameSession(sess, "named")) | |||
| 1047 | + | ||||
| 1048 | + | require.False(t, sess.ephemeral) | |||
| 1049 | + | require.Equal(t, "named", sess.name) | |||
| 1050 | + | require.Equal(t, int64(100), sess.createdAt) | |||
| 1051 | + | require.Equal(t, sess.createdAt, state.record(t, "named").CreatedAt) | |||
| 1052 | + | } | |||
| 1053 | + | ||||
| 1054 | + | func TestEphemeralPromotionLifecyclePreventsStaleSameNamePaletteTarget(t *testing.T) { | |||
| 1055 | + | fromPTY, releaseFrom := newBlockingPTY(t) | |||
| 1056 | + | firstPTY, releaseFirst := newBlockingPTY(t) | |||
| 1057 | + | secondPTY, releaseSecond := newBlockingPTY(t) | |||
| 1058 | + | defer releaseFrom() | |||
| 1059 | + | defer releaseFirst() | |||
| 1060 | + | defer releaseSecond() | |||
| 1061 | + | ||||
| 1062 | + | d, from, ac, _ := newManualSessionWithPTYs(t, fromPTY) | |||
| 1063 | + | store, _ := newMockStore(t) | |||
| 1064 | + | clock := &lifecycleClock{nows: []time.Time{time.Unix(0, 100), time.Unix(0, 100)}} | |||
| 1065 | + | d.clock = clock | |||
| 1066 | + | d.ptys = newFactorySeq(t, firstPTY, secondPTY) | |||
| 1067 | + | WithStore(store)(d) | |||
| 1068 | + | ||||
| 1069 | + | first, err := d.createSessionLocked("0", true, "/tmp", ac.size, terminalEnv{}, d.baseEnv) | |||
| 1070 | + | require.NoError(t, err) | |||
| 1071 | + | require.NoError(t, d.renameSession(first, "named")) | |||
| 1072 | + | staleCreatedAt := first.createdAt | |||
| 1073 | + | require.NotZero(t, staleCreatedAt) | |||
| 1074 | + | require.NoError(t, d.killSession(first, ports.ReasonSessionKilled, true)) | |||
| 1075 | + | d.sessWg.Wait() | |||
| 1076 | + | ||||
| 1077 | + | second, err := d.createSessionLocked("0", true, "/tmp", ac.size, terminalEnv{}, d.baseEnv) | |||
| 1078 | + | require.NoError(t, err) | |||
| 1079 | + | require.NoError(t, d.renameSession(second, "named")) | |||
| 1080 | + | require.NotEqual(t, staleCreatedAt, second.createdAt) | |||
| 1081 | + | ||||
| 1082 | + | require.False(t, d.switchToTarget(from, ac, picker.Target{Name: "named", TabIndex: -1, ExpectedCreatedAt: &staleCreatedAt})) | |||
| 1083 | + | require.Same(t, from, ac.currentSession()) | |||
| 1084 | + | releaseSecond() | |||
| 1085 | + | d.sessWg.Wait() | |||
| 1086 | + | } | |||
| 1087 | + | ||||
| 1088 | + | func TestEphemeralPromotionLifecycleFailuresLeaveStateRollbackSafe(t *testing.T) { | |||
| 1089 | + | t.Run("allocator exhaustion", func(t *testing.T) { | |||
| 1090 | + | p, release := newBlockingPTY(t) | |||
| 1091 | + | defer release() | |||
| 1092 | + | store, state := newMockStore(t) | |||
| 1093 | + | d := newTestDaemon(t, newFactory(t, p), stubClock{}) | |||
| 1094 | + | WithStore(store)(d) | |||
| 1095 | + | d.lastAllocatedCreatedAt = math.MaxInt64 | |||
| 1096 | + | ||||
| 1097 | + | sess, err := d.createSessionLocked("0", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, d.baseEnv) | |||
| 1098 | + | require.NoError(t, err) | |||
| 1099 | + | require.EqualError(t, d.renameSession(sess, "named"), "daemon: lifecycle identities exhausted") | |||
| 1100 | + | require.Equal(t, "0", sess.name) | |||
| 1101 | + | require.True(t, sess.ephemeral) | |||
| 1102 | + | require.Zero(t, sess.createdAt) | |||
| 1103 | + | require.Equal(t, int64(math.MaxInt64), d.lastAllocatedCreatedAt) | |||
| 1104 | + | require.False(t, state.has("named")) | |||
| 1105 | + | }) | |||
| 1106 | + | ||||
| 1107 | + | t.Run("persistence failure", func(t *testing.T) { | |||
| 1108 | + | p, release := newBlockingPTY(t) | |||
| 1109 | + | defer release() | |||
| 1110 | + | store := portsmocks.NewMockStore(t) | |||
| 1111 | + | var attempted map[string][]byte | |||
| 1112 | + | store.EXPECT().Set(mock.Anything, mock.Anything).RunAndReturn(func(key, value []byte) error { | |||
| 1113 | + | attempted = map[string][]byte{string(key): append([]byte(nil), value...)} | |||
| 1114 | + | return errors.New("disk full") | |||
| 1115 | + | }).Once() | |||
| 1116 | + | clock := &lifecycleClock{nows: []time.Time{time.Unix(0, 100), time.Unix(0, 100)}} | |||
| 1117 | + | d := newTestDaemon(t, newFactory(t, p), clock) | |||
| 1118 | + | WithStore(store)(d) | |||
| 1119 | + | ||||
| 1120 | + | sess, err := d.createSessionLocked("0", true, "/tmp", domain.Size{Cols: 80, Rows: 24}, terminalEnv{}, d.baseEnv) | |||
| 1121 | + | require.NoError(t, err) | |||
| 1122 | + | require.EqualError(t, d.renameSession(sess, "named"), "disk full") | |||
| 1123 | + | require.Equal(t, "0", sess.name) | |||
| 1124 | + | require.True(t, sess.ephemeral) | |||
| 1125 | + | require.Zero(t, sess.createdAt) | |||
| 1126 | + | require.Equal(t, int64(100), d.lastAllocatedCreatedAt, "a failed durable write may be ambiguous, so its lifecycle identity remains reserved") | |||
| 1127 | + | records, err := persist.New(newStaticStore(attempted)).LoadAll() | |||
| 1128 | + | require.NoError(t, err) | |||
| 1129 | + | require.Len(t, records, 1) | |||
| 1130 | + | require.Equal(t, int64(100), records[0].CreatedAt, "a promotion must never attempt to persist a zero lifecycle identity") | |||
| 1131 | + | ||||
| 1132 | + | store.EXPECT().Set(mock.Anything, mock.Anything).Return(nil).Once() | |||
| 1133 | + | store.EXPECT().Sync().Return(nil).Once() | |||
| 1134 | + | require.NoError(t, d.renameSession(sess, "named")) | |||
| 1135 | + | require.Equal(t, int64(101), sess.createdAt, "the high-water mark must remain monotonic after a failed promotion") | |||
| 1136 | + | }) | |||
| 1137 | + | } | |||
| 1138 | + | ||||
| 1033 | func TestRefreshSessionCwdTouchesOnlyOnChange(t *testing.T) { | 1139 | func TestRefreshSessionCwdTouchesOnlyOnChange(t *testing.T) { | ||
| 1034 | sz := domain.Size{Cols: 80, Rows: 24} | 1140 | sz := domain.Size{Cols: 80, Rows: 24} | ||
| 1035 | p, release := newBlockingPTY(t) | 1141 | p, release := newBlockingPTY(t) | ||