fix(notices): release claims and stop exited input
- commit
- 2250adb9d32dc41345c4fced76d8ee885d6beaaa
- author
- brice · · 2026-07-21 14:21 UTC
- parent
- d558968
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +201 -42
M internal/adapters/noticefile/noticefile.go +29 -7
| @@ -39,6 +39,11 @@ type Store struct { | |||
| 39 | 39 | // opened the pending file while it owns the interprocess lock. It is nil in | |
| 40 | 40 | // production. | |
| 41 | 41 | afterAppendOpen func() | |
| 42 | + | ||
| 43 | + | // removeFile and syncDirectory make Ack cleanup failures deterministic in | |
| 44 | + | // package tests. They are nil in production. | |
| 45 | + | removeFile func(string) error | |
| 46 | + | syncDirectory func(string) error | |
| 42 | 47 | } | |
| 43 | 48 | ||
| 44 | 49 | var ( | |
| @@ -192,13 +197,19 @@ func (s *Store) Append(n domain.Notification) error { | |||
| 192 | 197 | } | |
| 193 | 198 | ||
| 194 | 199 | // Claim atomically moves pending notices into an in-flight file and returns | |
| 195 | - | // their valid JSONL entries in order. Its Store owns the claim until Ack, so a | |
| 196 | - | // concurrent Store cannot replay or acknowledge it. The ownership flock is | |
| 197 | - | // released on process exit, allowing an unacknowledged import to be replayed | |
| 198 | - | // after a crash. A missing pending file means there is nothing to claim. | |
| 200 | + | // their valid JSONL entries in order. Repeating Claim on its owning Store | |
| 201 | + | // replays that Store's existing claim; a concurrent Store cannot replay or | |
| 202 | + | // acknowledge it. The ownership flock is released on process exit, allowing | |
| 203 | + | // an unacknowledged import to be replayed after a crash. A missing pending | |
| 204 | + | // file means there is nothing to claim. | |
| 199 | 205 | func (s *Store) Claim() ([]domain.Notification, error) { | |
| 200 | 206 | var notices []domain.Notification | |
| 201 | 207 | err := s.withLock(func() error { | |
| 208 | + | if s.ownsClaim() { | |
| 209 | + | var err error | |
| 210 | + | notices, err = readNotices(s.inFlightPath()) | |
| 211 | + | return err | |
| 212 | + | } | |
| 202 | 213 | if err := s.acquireClaimLock(); err != nil { | |
| 203 | 214 | return err | |
| 204 | 215 | } | |
| @@ -318,17 +329,28 @@ func (s *Store) Ack() error { | |||
| 318 | 329 | if !s.ownsClaim() { | |
| 319 | 330 | return ErrNoClaimOwner | |
| 320 | 331 | } | |
| 332 | + | // Ack is terminal for this Store's claim even when removing or syncing the | |
| 333 | + | // record fails: retaining the flock would indefinitely block recovery. | |
| 334 | + | defer s.releaseClaimLock() | |
| 335 | + | ||
| 321 | 336 | return s.withLock(func() error { | |
| 322 | 337 | if !s.ownsClaim() { | |
| 323 | 338 | return ErrNoClaimOwner | |
| 324 | 339 | } | |
| 325 | - | if err := os.Remove(s.inFlightPath()); err != nil && !errors.Is(err, os.ErrNotExist) { | |
| 340 | + | remove := os.Remove | |
| 341 | + | if s.removeFile != nil { | |
| 342 | + | remove = s.removeFile | |
| 343 | + | } | |
| 344 | + | if err := remove(s.inFlightPath()); err != nil && !errors.Is(err, os.ErrNotExist) { | |
| 326 | 345 | return fmt.Errorf("ack claimed notices: %w", err) | |
| 327 | 346 | } | |
| 328 | - | if err := syncDir(s.dir); err != nil { | |
| 347 | + | sync := syncDir | |
| 348 | + | if s.syncDirectory != nil { | |
| 349 | + | sync = s.syncDirectory | |
| 350 | + | } | |
| 351 | + | if err := sync(s.dir); err != nil { | |
| 329 | 352 | return fmt.Errorf("sync acknowledged notices: %w", err) | |
| 330 | 353 | } | |
| 331 | - | s.releaseClaimLock() | |
| 332 | 354 | return nil | |
| 333 | 355 | }) | |
| 334 | 356 | } | |
| @@ -39,6 +39,11 @@ type Store struct { | |||||
| 39 | // opened the pending file while it owns the interprocess lock. It is nil in | 39 | // opened the pending file while it owns the interprocess lock. It is nil in | ||
| 40 | // production. | 40 | // production. | ||
| 41 | afterAppendOpen func() | 41 | afterAppendOpen func() | ||
| 42 | + | ||||
| 43 | + | // removeFile and syncDirectory make Ack cleanup failures deterministic in | |||
| 44 | + | // package tests. They are nil in production. | |||
| 45 | + | removeFile func(string) error | |||
| 46 | + | syncDirectory func(string) error | |||
| 42 | } | 47 | } | ||
| 43 | 48 | ||||
| 44 | var ( | 49 | var ( | ||
| @@ -192,13 +197,19 @@ func (s *Store) Append(n domain.Notification) error { | |||||
| 192 | } | 197 | } | ||
| 193 | 198 | ||||
| 194 | // Claim atomically moves pending notices into an in-flight file and returns | 199 | // Claim atomically moves pending notices into an in-flight file and returns | ||
| 195 | - | // their valid JSONL entries in order. Its Store owns the claim until Ack, so a | 200 | + | // their valid JSONL entries in order. Repeating Claim on its owning Store |
| 196 | - | // concurrent Store cannot replay or acknowledge it. The ownership flock is | 201 | + | // replays that Store's existing claim; a concurrent Store cannot replay or |
| 197 | - | // released on process exit, allowing an unacknowledged import to be replayed | 202 | + | // acknowledge it. The ownership flock is released on process exit, allowing |
| 198 | - | // after a crash. A missing pending file means there is nothing to claim. | 203 | + | // an unacknowledged import to be replayed after a crash. A missing pending |
| 204 | + | // file means there is nothing to claim. | |||
| 199 | func (s *Store) Claim() ([]domain.Notification, error) { | 205 | func (s *Store) Claim() ([]domain.Notification, error) { | ||
| 200 | var notices []domain.Notification | 206 | var notices []domain.Notification | ||
| 201 | err := s.withLock(func() error { | 207 | err := s.withLock(func() error { | ||
| 208 | + | if s.ownsClaim() { | |||
| 209 | + | var err error | |||
| 210 | + | notices, err = readNotices(s.inFlightPath()) | |||
| 211 | + | return err | |||
| 212 | + | } | |||
| 202 | if err := s.acquireClaimLock(); err != nil { | 213 | if err := s.acquireClaimLock(); err != nil { | ||
| 203 | return err | 214 | return err | ||
| 204 | } | 215 | } | ||
| @@ -318,17 +329,28 @@ func (s *Store) Ack() error { | |||||
| 318 | if !s.ownsClaim() { | 329 | if !s.ownsClaim() { | ||
| 319 | return ErrNoClaimOwner | 330 | return ErrNoClaimOwner | ||
| 320 | } | 331 | } | ||
| 332 | + | // Ack is terminal for this Store's claim even when removing or syncing the | |||
| 333 | + | // record fails: retaining the flock would indefinitely block recovery. | |||
| 334 | + | defer s.releaseClaimLock() | |||
| 335 | + | ||||
| 321 | return s.withLock(func() error { | 336 | return s.withLock(func() error { | ||
| 322 | if !s.ownsClaim() { | 337 | if !s.ownsClaim() { | ||
| 323 | return ErrNoClaimOwner | 338 | return ErrNoClaimOwner | ||
| 324 | } | 339 | } | ||
| 325 | - | if err := os.Remove(s.inFlightPath()); err != nil && !errors.Is(err, os.ErrNotExist) { | 340 | + | remove := os.Remove |
| 341 | + | if s.removeFile != nil { | |||
| 342 | + | remove = s.removeFile | |||
| 343 | + | } | |||
| 344 | + | if err := remove(s.inFlightPath()); err != nil && !errors.Is(err, os.ErrNotExist) { | |||
| 326 | return fmt.Errorf("ack claimed notices: %w", err) | 345 | return fmt.Errorf("ack claimed notices: %w", err) | ||
| 327 | } | 346 | } | ||
| 328 | - | if err := syncDir(s.dir); err != nil { | 347 | + | sync := syncDir |
| 348 | + | if s.syncDirectory != nil { | |||
| 349 | + | sync = s.syncDirectory | |||
| 350 | + | } | |||
| 351 | + | if err := sync(s.dir); err != nil { | |||
| 329 | return fmt.Errorf("sync acknowledged notices: %w", err) | 352 | return fmt.Errorf("sync acknowledged notices: %w", err) | ||
| 330 | } | 353 | } | ||
| 331 | - | s.releaseClaimLock() | |||
| 332 | return nil | 354 | return nil | ||
| 333 | }) | 355 | }) | ||
| 334 | } | 356 | } | ||
M internal/adapters/noticefile/noticefile_test.go +69 -0
| @@ -2,6 +2,7 @@ package noticefile | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "encoding/json" | |
| 5 | + | "errors" | |
| 5 | 6 | "os" | |
| 6 | 7 | "path/filepath" | |
| 7 | 8 | "strings" | |
| @@ -88,6 +89,74 @@ func TestStoreClaimWaitsForAppendInCriticalSection(t *testing.T) { | |||
| 88 | 89 | require.NoError(t, claimant.Ack()) | |
| 89 | 90 | } | |
| 90 | 91 | ||
| 92 | + | func TestStoreRepeatedClaimReplaysOwnerAndBlocksCompetitor(t *testing.T) { | |
| 93 | + | t.Parallel() | |
| 94 | + | dir := filepath.Join(t.TempDir(), "vev") | |
| 95 | + | owner := New(dir) | |
| 96 | + | want := domain.Notification{Message: "owned"} | |
| 97 | + | require.NoError(t, owner.Append(want)) | |
| 98 | + | ||
| 99 | + | first, err := owner.Claim() | |
| 100 | + | require.NoError(t, err) | |
| 101 | + | require.Equal(t, []domain.Notification{want}, first) | |
| 102 | + | ||
| 103 | + | replayed, err := owner.Claim() | |
| 104 | + | require.NoError(t, err) | |
| 105 | + | require.Equal(t, first, replayed) | |
| 106 | + | ||
| 107 | + | other := New(dir) | |
| 108 | + | blocked, err := other.Claim() | |
| 109 | + | require.ErrorIs(t, err, ErrClaimInProgress) | |
| 110 | + | require.Nil(t, blocked) | |
| 111 | + | ||
| 112 | + | require.NoError(t, owner.Ack()) | |
| 113 | + | empty, err := other.Claim() | |
| 114 | + | require.NoError(t, err) | |
| 115 | + | require.Empty(t, empty) | |
| 116 | + | } | |
| 117 | + | ||
| 118 | + | func TestStoreAckReleasesClaimAfterCleanupFailure(t *testing.T) { | |
| 119 | + | t.Parallel() | |
| 120 | + | ||
| 121 | + | for _, tt := range []struct { | |
| 122 | + | name string | |
| 123 | + | inject func(*Store, error) | |
| 124 | + | want []domain.Notification | |
| 125 | + | }{ | |
| 126 | + | { | |
| 127 | + | name: "remove", | |
| 128 | + | inject: func(store *Store, cause error) { | |
| 129 | + | store.removeFile = func(string) error { return cause } | |
| 130 | + | }, | |
| 131 | + | want: []domain.Notification{{Message: "owned"}}, | |
| 132 | + | }, | |
| 133 | + | { | |
| 134 | + | name: "sync directory", | |
| 135 | + | inject: func(store *Store, cause error) { | |
| 136 | + | store.syncDirectory = func(string) error { return cause } | |
| 137 | + | }, | |
| 138 | + | want: nil, | |
| 139 | + | }, | |
| 140 | + | } { | |
| 141 | + | t.Run(tt.name, func(t *testing.T) { | |
| 142 | + | dir := filepath.Join(t.TempDir(), "vev") | |
| 143 | + | owner := New(dir) | |
| 144 | + | require.NoError(t, owner.Append(domain.Notification{Message: "owned"})) | |
| 145 | + | _, err := owner.Claim() | |
| 146 | + | require.NoError(t, err) | |
| 147 | + | ||
| 148 | + | cause := errors.New("cleanup failed") | |
| 149 | + | tt.inject(owner, cause) | |
| 150 | + | require.ErrorIs(t, owner.Ack(), cause) | |
| 151 | + | ||
| 152 | + | // A new Store proves the failed Ack released the ownership flock. | |
| 153 | + | recovered, err := New(dir).Claim() | |
| 154 | + | require.NoError(t, err) | |
| 155 | + | require.Equal(t, tt.want, recovered) | |
| 156 | + | }) | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | ||
| 91 | 160 | func TestStoreClaimOwnershipPreventsLiveReplayOrAck(t *testing.T) { | |
| 92 | 161 | t.Parallel() | |
| 93 | 162 | dir := filepath.Join(t.TempDir(), "vev") | |
| @@ -2,6 +2,7 @@ package noticefile | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "encoding/json" | 4 | "encoding/json" | ||
| 5 | + | "errors" | |||
| 5 | "os" | 6 | "os" | ||
| 6 | "path/filepath" | 7 | "path/filepath" | ||
| 7 | "strings" | 8 | "strings" | ||
| @@ -88,6 +89,74 @@ func TestStoreClaimWaitsForAppendInCriticalSection(t *testing.T) { | |||||
| 88 | require.NoError(t, claimant.Ack()) | 89 | require.NoError(t, claimant.Ack()) | ||
| 89 | } | 90 | } | ||
| 90 | 91 | ||||
| 92 | + | func TestStoreRepeatedClaimReplaysOwnerAndBlocksCompetitor(t *testing.T) { | |||
| 93 | + | t.Parallel() | |||
| 94 | + | dir := filepath.Join(t.TempDir(), "vev") | |||
| 95 | + | owner := New(dir) | |||
| 96 | + | want := domain.Notification{Message: "owned"} | |||
| 97 | + | require.NoError(t, owner.Append(want)) | |||
| 98 | + | ||||
| 99 | + | first, err := owner.Claim() | |||
| 100 | + | require.NoError(t, err) | |||
| 101 | + | require.Equal(t, []domain.Notification{want}, first) | |||
| 102 | + | ||||
| 103 | + | replayed, err := owner.Claim() | |||
| 104 | + | require.NoError(t, err) | |||
| 105 | + | require.Equal(t, first, replayed) | |||
| 106 | + | ||||
| 107 | + | other := New(dir) | |||
| 108 | + | blocked, err := other.Claim() | |||
| 109 | + | require.ErrorIs(t, err, ErrClaimInProgress) | |||
| 110 | + | require.Nil(t, blocked) | |||
| 111 | + | ||||
| 112 | + | require.NoError(t, owner.Ack()) | |||
| 113 | + | empty, err := other.Claim() | |||
| 114 | + | require.NoError(t, err) | |||
| 115 | + | require.Empty(t, empty) | |||
| 116 | + | } | |||
| 117 | + | ||||
| 118 | + | func TestStoreAckReleasesClaimAfterCleanupFailure(t *testing.T) { | |||
| 119 | + | t.Parallel() | |||
| 120 | + | ||||
| 121 | + | for _, tt := range []struct { | |||
| 122 | + | name string | |||
| 123 | + | inject func(*Store, error) | |||
| 124 | + | want []domain.Notification | |||
| 125 | + | }{ | |||
| 126 | + | { | |||
| 127 | + | name: "remove", | |||
| 128 | + | inject: func(store *Store, cause error) { | |||
| 129 | + | store.removeFile = func(string) error { return cause } | |||
| 130 | + | }, | |||
| 131 | + | want: []domain.Notification{{Message: "owned"}}, | |||
| 132 | + | }, | |||
| 133 | + | { | |||
| 134 | + | name: "sync directory", | |||
| 135 | + | inject: func(store *Store, cause error) { | |||
| 136 | + | store.syncDirectory = func(string) error { return cause } | |||
| 137 | + | }, | |||
| 138 | + | want: nil, | |||
| 139 | + | }, | |||
| 140 | + | } { | |||
| 141 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 142 | + | dir := filepath.Join(t.TempDir(), "vev") | |||
| 143 | + | owner := New(dir) | |||
| 144 | + | require.NoError(t, owner.Append(domain.Notification{Message: "owned"})) | |||
| 145 | + | _, err := owner.Claim() | |||
| 146 | + | require.NoError(t, err) | |||
| 147 | + | ||||
| 148 | + | cause := errors.New("cleanup failed") | |||
| 149 | + | tt.inject(owner, cause) | |||
| 150 | + | require.ErrorIs(t, owner.Ack(), cause) | |||
| 151 | + | ||||
| 152 | + | // A new Store proves the failed Ack released the ownership flock. | |||
| 153 | + | recovered, err := New(dir).Claim() | |||
| 154 | + | require.NoError(t, err) | |||
| 155 | + | require.Equal(t, tt.want, recovered) | |||
| 156 | + | }) | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | ||||
| 91 | func TestStoreClaimOwnershipPreventsLiveReplayOrAck(t *testing.T) { | 160 | func TestStoreClaimOwnershipPreventsLiveReplayOrAck(t *testing.T) { | ||
| 92 | t.Parallel() | 161 | t.Parallel() | ||
| 93 | dir := filepath.Join(t.TempDir(), "vev") | 162 | dir := filepath.Join(t.TempDir(), "vev") | ||
M internal/domain/notice.go +4 -0
| @@ -41,6 +41,10 @@ const ( | |||
| 41 | 41 | NoticeClipboardTooLarge | |
| 42 | 42 | NoticeAutoResume | |
| 43 | 43 | NoticeConnection | |
| 44 | + | ||
| 45 | + | // noticeCodeLimit is an append-only declaration sentinel, not a persisted | |
| 46 | + | // valid notice code. Keep it last when adding a NoticeCode. | |
| 47 | + | noticeCodeLimit | |
| 44 | 48 | ) | |
| 45 | 49 | ||
| 46 | 50 | var noticeSlugs = map[NoticeCode]string{ | |
| @@ -41,6 +41,10 @@ const ( | |||||
| 41 | NoticeClipboardTooLarge | 41 | NoticeClipboardTooLarge | ||
| 42 | NoticeAutoResume | 42 | NoticeAutoResume | ||
| 43 | NoticeConnection | 43 | NoticeConnection | ||
| 44 | + | ||||
| 45 | + | // noticeCodeLimit is an append-only declaration sentinel, not a persisted | |||
| 46 | + | // valid notice code. Keep it last when adding a NoticeCode. | |||
| 47 | + | noticeCodeLimit | |||
| 44 | ) | 48 | ) | ||
| 45 | 49 | ||||
| 46 | var noticeSlugs = map[NoticeCode]string{ | 50 | var noticeSlugs = map[NoticeCode]string{ | ||
M internal/domain/notice_test.go +6 -29
| @@ -7,37 +7,14 @@ import ( | |||
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | 9 | func TestNoticeCodeString(t *testing.T) { | |
| 10 | - | tests := []struct { | |
| 11 | - | code NoticeCode | |
| 12 | - | want string | |
| 13 | - | }{ | |
| 14 | - | {NoticeInternal, "internal"}, | |
| 15 | - | {NoticePaneSpawn, "pane-spawn"}, | |
| 16 | - | {NoticeTabSpawn, "tab-spawn"}, | |
| 17 | - | {NoticeFloatingSpawn, "floating-spawn"}, | |
| 18 | - | {NoticeSessionSpawn, "session-spawn"}, | |
| 19 | - | {NoticeLayoutTooSmall, "layout-too-small"}, | |
| 20 | - | {NoticePaneNotFound, "pane-not-found"}, | |
| 21 | - | {NoticeSessionUnavailable, "session-unavailable"}, | |
| 22 | - | {NoticePersistDisabled, "persist-disabled"}, | |
| 23 | - | {NoticeSnapshotWrite, "snapshot-write"}, | |
| 24 | - | {NoticeSnapshotRestore, "snapshot-restore"}, | |
| 25 | - | {NoticeSnapshotSaturated, "snapshot-saturated"}, | |
| 26 | - | {NoticePersistDelete, "persist-delete"}, | |
| 27 | - | {NoticeConfigReload, "config-reload"}, | |
| 28 | - | {NoticeInputDropped, "input-dropped"}, | |
| 29 | - | {NoticeResizeFailed, "resize-failed"}, | |
| 30 | - | {NoticeClipboard, "clipboard"}, | |
| 31 | - | {NoticeClipboardTooLarge, "clipboard-too-large"}, | |
| 32 | - | {NoticeAutoResume, "auto-resume"}, | |
| 33 | - | {NoticeConnection, "connection"}, | |
| 34 | - | {NoticeCode(9999), "unknown"}, | |
| 35 | - | } | |
| 36 | - | for _, tt := range tests { | |
| 37 | - | if got := tt.code.String(); got != tt.want { | |
| 38 | - | t.Errorf("NoticeCode(%d).String() = %q, want %q", tt.code, got, tt.want) | |
| 10 | + | for code := range noticeCodeLimit { | |
| 11 | + | if got := code.String(); got == "unknown" { | |
| 12 | + | t.Errorf("NoticeCode(%d).String() = unknown, want declared code slug", code) | |
| 39 | 13 | } | |
| 40 | 14 | } | |
| 15 | + | if got := NoticeCode(9999).String(); got != "unknown" { | |
| 16 | + | t.Errorf("NoticeCode(9999).String() = %q, want unknown", got) | |
| 17 | + | } | |
| 41 | 18 | } | |
| 42 | 19 | ||
| 43 | 20 | func TestUserError(t *testing.T) { | |
| @@ -7,37 +7,14 @@ import ( | |||||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | func TestNoticeCodeString(t *testing.T) { | 9 | func TestNoticeCodeString(t *testing.T) { | ||
| 10 | - | tests := []struct { | 10 | + | for code := range noticeCodeLimit { |
| 11 | - | code NoticeCode | 11 | + | if got := code.String(); got == "unknown" { |
| 12 | - | want string | 12 | + | t.Errorf("NoticeCode(%d).String() = unknown, want declared code slug", code) |
| 13 | - | }{ | |||
| 14 | - | {NoticeInternal, "internal"}, | |||
| 15 | - | {NoticePaneSpawn, "pane-spawn"}, | |||
| 16 | - | {NoticeTabSpawn, "tab-spawn"}, | |||
| 17 | - | {NoticeFloatingSpawn, "floating-spawn"}, | |||
| 18 | - | {NoticeSessionSpawn, "session-spawn"}, | |||
| 19 | - | {NoticeLayoutTooSmall, "layout-too-small"}, | |||
| 20 | - | {NoticePaneNotFound, "pane-not-found"}, | |||
| 21 | - | {NoticeSessionUnavailable, "session-unavailable"}, | |||
| 22 | - | {NoticePersistDisabled, "persist-disabled"}, | |||
| 23 | - | {NoticeSnapshotWrite, "snapshot-write"}, | |||
| 24 | - | {NoticeSnapshotRestore, "snapshot-restore"}, | |||
| 25 | - | {NoticeSnapshotSaturated, "snapshot-saturated"}, | |||
| 26 | - | {NoticePersistDelete, "persist-delete"}, | |||
| 27 | - | {NoticeConfigReload, "config-reload"}, | |||
| 28 | - | {NoticeInputDropped, "input-dropped"}, | |||
| 29 | - | {NoticeResizeFailed, "resize-failed"}, | |||
| 30 | - | {NoticeClipboard, "clipboard"}, | |||
| 31 | - | {NoticeClipboardTooLarge, "clipboard-too-large"}, | |||
| 32 | - | {NoticeAutoResume, "auto-resume"}, | |||
| 33 | - | {NoticeConnection, "connection"}, | |||
| 34 | - | {NoticeCode(9999), "unknown"}, | |||
| 35 | - | } | |||
| 36 | - | for _, tt := range tests { | |||
| 37 | - | if got := tt.code.String(); got != tt.want { | |||
| 38 | - | t.Errorf("NoticeCode(%d).String() = %q, want %q", tt.code, got, tt.want) | |||
| 39 | } | 13 | } | ||
| 40 | } | 14 | } | ||
| 15 | + | if got := NoticeCode(9999).String(); got != "unknown" { | |||
| 16 | + | t.Errorf("NoticeCode(9999).String() = %q, want unknown", got) | |||
| 17 | + | } | |||
| 41 | } | 18 | } | ||
| 42 | 19 | ||||
| 43 | func TestUserError(t *testing.T) { | 20 | func TestUserError(t *testing.T) { | ||
M internal/ports/notice.go +4 -3
| @@ -5,9 +5,10 @@ import "github.com/bnema/vev/internal/domain" | |||
| 5 | 5 | // NoticeStore persists undeliverable notices across daemon restarts. | |
| 6 | 6 | type NoticeStore interface { | |
| 7 | 7 | Append(n domain.Notification) error | |
| 8 | - | // Claim atomically reserves pending notices for import. A live claim cannot | |
| 9 | - | // be replayed or acknowledged by another store; it remains recoverable after | |
| 10 | - | // the claimant process crashes, until its owning store calls Ack. | |
| 8 | + | // Claim atomically reserves pending notices for import. Repeating Claim on | |
| 9 | + | // the owning store replays its live claim; another store cannot replay or | |
| 10 | + | // acknowledge it. The claim remains recoverable after the claimant process | |
| 11 | + | // crashes, until its owning store calls Ack. | |
| 11 | 12 | Claim() ([]domain.Notification, error) | |
| 12 | 13 | // Ack permanently removes the notices returned by this store's claim. | |
| 13 | 14 | Ack() error | |
| @@ -5,9 +5,10 @@ import "github.com/bnema/vev/internal/domain" | |||||
| 5 | // NoticeStore persists undeliverable notices across daemon restarts. | 5 | // NoticeStore persists undeliverable notices across daemon restarts. | ||
| 6 | type NoticeStore interface { | 6 | type NoticeStore interface { | ||
| 7 | Append(n domain.Notification) error | 7 | Append(n domain.Notification) error | ||
| 8 | - | // Claim atomically reserves pending notices for import. A live claim cannot | 8 | + | // Claim atomically reserves pending notices for import. Repeating Claim on |
| 9 | - | // be replayed or acknowledged by another store; it remains recoverable after | 9 | + | // the owning store replays its live claim; another store cannot replay or |
| 10 | - | // the claimant process crashes, until its owning store calls Ack. | 10 | + | // acknowledge it. The claim remains recoverable after the claimant process |
| 11 | + | // crashes, until its owning store calls Ack. | |||
| 11 | Claim() ([]domain.Notification, error) | 12 | Claim() ([]domain.Notification, error) | ||
| 12 | // Ack permanently removes the notices returned by this store's claim. | 13 | // Ack permanently removes the notices returned by this store's claim. | ||
| 13 | Ack() error | 14 | Ack() error | ||
M internal/usecase/daemon/notices_overlay.go +9 -3
| @@ -71,6 +71,7 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||
| 71 | 71 | result.changed = true | |
| 72 | 72 | case 'q', 0x03: | |
| 73 | 73 | result.exit = true | |
| 74 | + | return result | |
| 74 | 75 | case keys.ESC: | |
| 75 | 76 | tail := data[i:] | |
| 76 | 77 | if consumed, move := routeListEscape(tail); consumed > 0 { | |
| @@ -100,9 +101,14 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||
| 100 | 101 | if custom.action != 0 { | |
| 101 | 102 | result.action = custom.action | |
| 102 | 103 | } | |
| 103 | - | result.exit = result.exit || custom.exit | |
| 104 | - | result.stop = custom.stop | |
| 105 | - | if result.stop { | |
| 104 | + | if custom.exit { | |
| 105 | + | result.exit = true | |
| 106 | + | } | |
| 107 | + | if custom.stop { | |
| 108 | + | result.stop = true | |
| 109 | + | return result | |
| 110 | + | } | |
| 111 | + | if custom.exit { | |
| 106 | 112 | return result | |
| 107 | 113 | } | |
| 108 | 114 | } | |
| @@ -71,6 +71,7 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||||
| 71 | result.changed = true | 71 | result.changed = true | ||
| 72 | case 'q', 0x03: | 72 | case 'q', 0x03: | ||
| 73 | result.exit = true | 73 | result.exit = true | ||
| 74 | + | return result | |||
| 74 | case keys.ESC: | 75 | case keys.ESC: | ||
| 75 | tail := data[i:] | 76 | tail := data[i:] | ||
| 76 | if consumed, move := routeListEscape(tail); consumed > 0 { | 77 | if consumed, move := routeListEscape(tail); consumed > 0 { | ||
| @@ -100,9 +101,14 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||||
| 100 | if custom.action != 0 { | 101 | if custom.action != 0 { | ||
| 101 | result.action = custom.action | 102 | result.action = custom.action | ||
| 102 | } | 103 | } | ||
| 103 | - | result.exit = result.exit || custom.exit | 104 | + | if custom.exit { |
| 104 | - | result.stop = custom.stop | 105 | + | result.exit = true |
| 105 | - | if result.stop { | 106 | + | } |
| 107 | + | if custom.stop { | |||
| 108 | + | result.stop = true | |||
| 109 | + | return result | |||
| 110 | + | } | |||
| 111 | + | if custom.exit { | |||
| 106 | return result | 112 | return result | ||
| 107 | } | 113 | } | ||
| 108 | } | 114 | } | ||
M internal/usecase/daemon/notices_overlay_test.go +80 -0
| @@ -222,6 +222,86 @@ func TestHandleListInputConsumesUnsupportedCompleteEscapeSequences(t *testing.T) | |||
| 222 | 222 | } | |
| 223 | 223 | } | |
| 224 | 224 | ||
| 225 | + | func TestHandleListInputStopsAfterExitInBatchedInput(t *testing.T) { | |
| 226 | + | for _, tt := range []struct { | |
| 227 | + | name string | |
| 228 | + | input []byte | |
| 229 | + | custom func(byte) listInputResult | |
| 230 | + | wantExit bool | |
| 231 | + | wantStop bool | |
| 232 | + | wantAction byte | |
| 233 | + | wantCalls []byte | |
| 234 | + | }{ | |
| 235 | + | { | |
| 236 | + | name: "q ignores trailing navigation and actions", | |
| 237 | + | input: []byte("qjy"), | |
| 238 | + | custom: func(b byte) listInputResult { return listInputResult{action: b} }, | |
| 239 | + | wantExit: true, | |
| 240 | + | }, | |
| 241 | + | { | |
| 242 | + | name: "ctrl-c ignores trailing navigation and actions", | |
| 243 | + | input: []byte{0x03, 'j', 'y'}, | |
| 244 | + | custom: func(b byte) listInputResult { return listInputResult{action: b} }, | |
| 245 | + | wantExit: true, | |
| 246 | + | }, | |
| 247 | + | { | |
| 248 | + | name: "custom exit retains action and ignores trailing bytes", | |
| 249 | + | input: []byte("xjy"), | |
| 250 | + | custom: func(b byte) listInputResult { | |
| 251 | + | if b == 'x' { | |
| 252 | + | return listInputResult{action: b, exit: true} | |
| 253 | + | } | |
| 254 | + | return listInputResult{action: b} | |
| 255 | + | }, | |
| 256 | + | wantExit: true, wantAction: 'x', wantCalls: []byte("x"), | |
| 257 | + | }, | |
| 258 | + | { | |
| 259 | + | name: "custom stop retains action and ignores trailing bytes", | |
| 260 | + | input: []byte("xjy"), | |
| 261 | + | custom: func(b byte) listInputResult { | |
| 262 | + | if b == 'x' { | |
| 263 | + | return listInputResult{action: b, stop: true} | |
| 264 | + | } | |
| 265 | + | return listInputResult{action: b} | |
| 266 | + | }, | |
| 267 | + | wantStop: true, wantAction: 'x', wantCalls: []byte("x"), | |
| 268 | + | }, | |
| 269 | + | { | |
| 270 | + | name: "custom exit and stop retain both flags and action", | |
| 271 | + | input: []byte("xjy"), | |
| 272 | + | custom: func(b byte) listInputResult { | |
| 273 | + | if b == 'x' { | |
| 274 | + | return listInputResult{action: b, exit: true, stop: true} | |
| 275 | + | } | |
| 276 | + | return listInputResult{action: b} | |
| 277 | + | }, | |
| 278 | + | wantExit: true, wantStop: true, wantAction: 'x', wantCalls: []byte("x"), | |
| 279 | + | }, | |
| 280 | + | } { | |
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |
| 282 | + | var pending, calls []byte | |
| 283 | + | var down int | |
| 284 | + | state := listInputState{ | |
| 285 | + | pending: &pending, | |
| 286 | + | esc: &pendingByteTimer{}, | |
| 287 | + | moveUp: func() {}, | |
| 288 | + | moveDown: func() { down++ }, | |
| 289 | + | } | |
| 290 | + | ||
| 291 | + | result := handleListInputLocked(nil, tt.input, state, func(b byte) listInputResult { | |
| 292 | + | calls = append(calls, b) | |
| 293 | + | return tt.custom(b) | |
| 294 | + | }) | |
| 295 | + | ||
| 296 | + | require.Equal(t, tt.wantExit, result.exit) | |
| 297 | + | require.Equal(t, tt.wantStop, result.stop) | |
| 298 | + | require.Equal(t, tt.wantAction, result.action) | |
| 299 | + | require.Equal(t, tt.wantCalls, calls) | |
| 300 | + | require.Zero(t, down, "trailing j must not be processed") | |
| 301 | + | }) | |
| 302 | + | } | |
| 303 | + | } | |
| 304 | + | ||
| 225 | 305 | func TestNoticesOpenWithEmptyHistoryShowsPlaceholderWithoutPanic(t *testing.T) { | |
| 226 | 306 | p, release := newBlockingPTY(t) | |
| 227 | 307 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| @@ -222,6 +222,86 @@ func TestHandleListInputConsumesUnsupportedCompleteEscapeSequences(t *testing.T) | |||||
| 222 | } | 222 | } | ||
| 223 | } | 223 | } | ||
| 224 | 224 | ||||
| 225 | + | func TestHandleListInputStopsAfterExitInBatchedInput(t *testing.T) { | |||
| 226 | + | for _, tt := range []struct { | |||
| 227 | + | name string | |||
| 228 | + | input []byte | |||
| 229 | + | custom func(byte) listInputResult | |||
| 230 | + | wantExit bool | |||
| 231 | + | wantStop bool | |||
| 232 | + | wantAction byte | |||
| 233 | + | wantCalls []byte | |||
| 234 | + | }{ | |||
| 235 | + | { | |||
| 236 | + | name: "q ignores trailing navigation and actions", | |||
| 237 | + | input: []byte("qjy"), | |||
| 238 | + | custom: func(b byte) listInputResult { return listInputResult{action: b} }, | |||
| 239 | + | wantExit: true, | |||
| 240 | + | }, | |||
| 241 | + | { | |||
| 242 | + | name: "ctrl-c ignores trailing navigation and actions", | |||
| 243 | + | input: []byte{0x03, 'j', 'y'}, | |||
| 244 | + | custom: func(b byte) listInputResult { return listInputResult{action: b} }, | |||
| 245 | + | wantExit: true, | |||
| 246 | + | }, | |||
| 247 | + | { | |||
| 248 | + | name: "custom exit retains action and ignores trailing bytes", | |||
| 249 | + | input: []byte("xjy"), | |||
| 250 | + | custom: func(b byte) listInputResult { | |||
| 251 | + | if b == 'x' { | |||
| 252 | + | return listInputResult{action: b, exit: true} | |||
| 253 | + | } | |||
| 254 | + | return listInputResult{action: b} | |||
| 255 | + | }, | |||
| 256 | + | wantExit: true, wantAction: 'x', wantCalls: []byte("x"), | |||
| 257 | + | }, | |||
| 258 | + | { | |||
| 259 | + | name: "custom stop retains action and ignores trailing bytes", | |||
| 260 | + | input: []byte("xjy"), | |||
| 261 | + | custom: func(b byte) listInputResult { | |||
| 262 | + | if b == 'x' { | |||
| 263 | + | return listInputResult{action: b, stop: true} | |||
| 264 | + | } | |||
| 265 | + | return listInputResult{action: b} | |||
| 266 | + | }, | |||
| 267 | + | wantStop: true, wantAction: 'x', wantCalls: []byte("x"), | |||
| 268 | + | }, | |||
| 269 | + | { | |||
| 270 | + | name: "custom exit and stop retain both flags and action", | |||
| 271 | + | input: []byte("xjy"), | |||
| 272 | + | custom: func(b byte) listInputResult { | |||
| 273 | + | if b == 'x' { | |||
| 274 | + | return listInputResult{action: b, exit: true, stop: true} | |||
| 275 | + | } | |||
| 276 | + | return listInputResult{action: b} | |||
| 277 | + | }, | |||
| 278 | + | wantExit: true, wantStop: true, wantAction: 'x', wantCalls: []byte("x"), | |||
| 279 | + | }, | |||
| 280 | + | } { | |||
| 281 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 282 | + | var pending, calls []byte | |||
| 283 | + | var down int | |||
| 284 | + | state := listInputState{ | |||
| 285 | + | pending: &pending, | |||
| 286 | + | esc: &pendingByteTimer{}, | |||
| 287 | + | moveUp: func() {}, | |||
| 288 | + | moveDown: func() { down++ }, | |||
| 289 | + | } | |||
| 290 | + | ||||
| 291 | + | result := handleListInputLocked(nil, tt.input, state, func(b byte) listInputResult { | |||
| 292 | + | calls = append(calls, b) | |||
| 293 | + | return tt.custom(b) | |||
| 294 | + | }) | |||
| 295 | + | ||||
| 296 | + | require.Equal(t, tt.wantExit, result.exit) | |||
| 297 | + | require.Equal(t, tt.wantStop, result.stop) | |||
| 298 | + | require.Equal(t, tt.wantAction, result.action) | |||
| 299 | + | require.Equal(t, tt.wantCalls, calls) | |||
| 300 | + | require.Zero(t, down, "trailing j must not be processed") | |||
| 301 | + | }) | |||
| 302 | + | } | |||
| 303 | + | } | |||
| 304 | + | ||||
| 225 | func TestNoticesOpenWithEmptyHistoryShowsPlaceholderWithoutPanic(t *testing.T) { | 305 | func TestNoticesOpenWithEmptyHistoryShowsPlaceholderWithoutPanic(t *testing.T) { | ||
| 226 | p, release := newBlockingPTY(t) | 306 | p, release := newBlockingPTY(t) | ||
| 227 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | 307 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | ||