fix(persistence): resolve follow-up PR comments
- commit
- b5b098c903333b5d37be9b9d984ca68c547835fd
- author
- brice · · 2026-07-03 21:20 UTC
- parent
- aead79d
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +49 -25
M internal/app/run.go +18 -8
| @@ -565,19 +565,29 @@ func runKill(_ context.Context, name string, all, daemon bool) error { | |||
| 565 | 565 | } else if statErr != nil { | |
| 566 | 566 | return fmt.Errorf("vev: reading stored sessions: %w", statErr) | |
| 567 | 567 | } | |
| 568 | - | store, openErr := kv.Open(storePath) | |
| 569 | - | if openErr != nil { | |
| 570 | - | return fmt.Errorf("vev: opening stored sessions: %w", openErr) | |
| 568 | + | records, loadErr := persist.LoadReadOnly(platform.StateDir()) | |
| 569 | + | if loadErr != nil { | |
| 570 | + | return fmt.Errorf("vev: reading stored sessions: %w", loadErr) | |
| 571 | + | } | |
| 572 | + | found := false | |
| 573 | + | for _, record := range records { | |
| 574 | + | if record.Name == name { | |
| 575 | + | found = true | |
| 576 | + | break | |
| 577 | + | } | |
| 571 | 578 | } | |
| 572 | - | if _, ok := store.Get([]byte(name)); !ok { | |
| 573 | - | _ = store.Close() | |
| 579 | + | if !found { | |
| 574 | 580 | return fmt.Errorf("vev: no such session: %s", name) | |
| 575 | 581 | } | |
| 576 | - | if deleteErr := store.Delete([]byte(name)); deleteErr != nil { | |
| 577 | - | _ = store.Close() | |
| 582 | + | p, openErr := persist.Open(platform.StateDir()) | |
| 583 | + | if openErr != nil { | |
| 584 | + | return fmt.Errorf("vev: opening stored sessions: %w", openErr) | |
| 585 | + | } | |
| 586 | + | if deleteErr := p.Delete(name); deleteErr != nil { | |
| 587 | + | _ = p.Close() | |
| 578 | 588 | return fmt.Errorf("vev: deleting stored session: %w", deleteErr) | |
| 579 | 589 | } | |
| 580 | - | if closeErr := store.Close(); closeErr != nil { | |
| 590 | + | if closeErr := p.Close(); closeErr != nil { | |
| 581 | 591 | return fmt.Errorf("vev: closing stored sessions: %w", closeErr) | |
| 582 | 592 | } | |
| 583 | 593 | printKillSuccess(name, all, daemon) | |
| @@ -565,19 +565,29 @@ func runKill(_ context.Context, name string, all, daemon bool) error { | |||||
| 565 | } else if statErr != nil { | 565 | } else if statErr != nil { | ||
| 566 | return fmt.Errorf("vev: reading stored sessions: %w", statErr) | 566 | return fmt.Errorf("vev: reading stored sessions: %w", statErr) | ||
| 567 | } | 567 | } | ||
| 568 | - | store, openErr := kv.Open(storePath) | 568 | + | records, loadErr := persist.LoadReadOnly(platform.StateDir()) |
| 569 | - | if openErr != nil { | 569 | + | if loadErr != nil { |
| 570 | - | return fmt.Errorf("vev: opening stored sessions: %w", openErr) | 570 | + | return fmt.Errorf("vev: reading stored sessions: %w", loadErr) |
| 571 | + | } | |||
| 572 | + | found := false | |||
| 573 | + | for _, record := range records { | |||
| 574 | + | if record.Name == name { | |||
| 575 | + | found = true | |||
| 576 | + | break | |||
| 577 | + | } | |||
| 571 | } | 578 | } | ||
| 572 | - | if _, ok := store.Get([]byte(name)); !ok { | 579 | + | if !found { |
| 573 | - | _ = store.Close() | |||
| 574 | return fmt.Errorf("vev: no such session: %s", name) | 580 | return fmt.Errorf("vev: no such session: %s", name) | ||
| 575 | } | 581 | } | ||
| 576 | - | if deleteErr := store.Delete([]byte(name)); deleteErr != nil { | 582 | + | p, openErr := persist.Open(platform.StateDir()) |
| 577 | - | _ = store.Close() | 583 | + | if openErr != nil { |
| 584 | + | return fmt.Errorf("vev: opening stored sessions: %w", openErr) | |||
| 585 | + | } | |||
| 586 | + | if deleteErr := p.Delete(name); deleteErr != nil { | |||
| 587 | + | _ = p.Close() | |||
| 578 | return fmt.Errorf("vev: deleting stored session: %w", deleteErr) | 588 | return fmt.Errorf("vev: deleting stored session: %w", deleteErr) | ||
| 579 | } | 589 | } | ||
| 580 | - | if closeErr := store.Close(); closeErr != nil { | 590 | + | if closeErr := p.Close(); closeErr != nil { |
| 581 | return fmt.Errorf("vev: closing stored sessions: %w", closeErr) | 591 | return fmt.Errorf("vev: closing stored sessions: %w", closeErr) | ||
| 582 | } | 592 | } | ||
| 583 | printKillSuccess(name, all, daemon) | 593 | printKillSuccess(name, all, daemon) | ||
M internal/app/run_test.go +8 -8
| @@ -202,18 +202,18 @@ func captureStdout(t *testing.T, fn func()) string { | |||
| 202 | 202 | os.Stdout = w | |
| 203 | 203 | defer func() { os.Stdout = old }() | |
| 204 | 204 | defer func() { _ = r.Close() }() | |
| 205 | - | closed := false | |
| 206 | - | defer func() { | |
| 207 | - | if !closed { | |
| 208 | - | _ = w.Close() | |
| 209 | - | } | |
| 205 | + | outCh := make(chan []byte, 1) | |
| 206 | + | errCh := make(chan error, 1) | |
| 207 | + | go func() { | |
| 208 | + | out, err := io.ReadAll(r) | |
| 209 | + | outCh <- out | |
| 210 | + | errCh <- err | |
| 210 | 211 | }() | |
| 211 | 212 | ||
| 212 | 213 | fn() | |
| 213 | 214 | _ = w.Close() | |
| 214 | - | closed = true | |
| 215 | - | out, err := io.ReadAll(r) | |
| 216 | - | if err != nil { | |
| 215 | + | out := <-outCh | |
| 216 | + | if err := <-errCh; err != nil { | |
| 217 | 217 | t.Fatalf("ReadAll stdout error = %v", err) | |
| 218 | 218 | } | |
| 219 | 219 | return string(out) | |
| @@ -202,18 +202,18 @@ func captureStdout(t *testing.T, fn func()) string { | |||||
| 202 | os.Stdout = w | 202 | os.Stdout = w | ||
| 203 | defer func() { os.Stdout = old }() | 203 | defer func() { os.Stdout = old }() | ||
| 204 | defer func() { _ = r.Close() }() | 204 | defer func() { _ = r.Close() }() | ||
| 205 | - | closed := false | 205 | + | outCh := make(chan []byte, 1) |
| 206 | - | defer func() { | 206 | + | errCh := make(chan error, 1) |
| 207 | - | if !closed { | 207 | + | go func() { |
| 208 | - | _ = w.Close() | 208 | + | out, err := io.ReadAll(r) |
| 209 | - | } | 209 | + | outCh <- out |
| 210 | + | errCh <- err | |||
| 210 | }() | 211 | }() | ||
| 211 | 212 | ||||
| 212 | fn() | 213 | fn() | ||
| 213 | _ = w.Close() | 214 | _ = w.Close() | ||
| 214 | - | closed = true | 215 | + | out := <-outCh |
| 215 | - | out, err := io.ReadAll(r) | 216 | + | if err := <-errCh; err != nil { |
| 216 | - | if err != nil { | |||
| 217 | t.Fatalf("ReadAll stdout error = %v", err) | 217 | t.Fatalf("ReadAll stdout error = %v", err) | ||
| 218 | } | 218 | } | ||
| 219 | return string(out) | 219 | return string(out) | ||
M internal/persist/record.go +7 -2
| @@ -44,8 +44,13 @@ func decodeRecordValue(name string, value []byte) (Record, error) { | |||
| 44 | 44 | if len(value) < 4 { | |
| 45 | 45 | return Record{}, errMalformedRecord | |
| 46 | 46 | } | |
| 47 | - | cwdLen := int(binary.BigEndian.Uint32(value[:4])) | |
| 48 | - | if cwdLen > len(value)-4 || len(value) != 4+cwdLen+8+8 { | |
| 47 | + | cwdLen32 := binary.BigEndian.Uint32(value[:4]) | |
| 48 | + | remaining := len(value) - 4 | |
| 49 | + | if uint64(cwdLen32) > uint64(remaining) { | |
| 50 | + | return Record{}, errMalformedRecord | |
| 51 | + | } | |
| 52 | + | cwdLen := int(cwdLen32) | |
| 53 | + | if len(value) != 4+cwdLen+8+8 { | |
| 49 | 54 | return Record{}, errMalformedRecord | |
| 50 | 55 | } | |
| 51 | 56 | off := 4 + cwdLen | |
| @@ -44,8 +44,13 @@ func decodeRecordValue(name string, value []byte) (Record, error) { | |||||
| 44 | if len(value) < 4 { | 44 | if len(value) < 4 { | ||
| 45 | return Record{}, errMalformedRecord | 45 | return Record{}, errMalformedRecord | ||
| 46 | } | 46 | } | ||
| 47 | - | cwdLen := int(binary.BigEndian.Uint32(value[:4])) | 47 | + | cwdLen32 := binary.BigEndian.Uint32(value[:4]) |
| 48 | - | if cwdLen > len(value)-4 || len(value) != 4+cwdLen+8+8 { | 48 | + | remaining := len(value) - 4 |
| 49 | + | if uint64(cwdLen32) > uint64(remaining) { | |||
| 50 | + | return Record{}, errMalformedRecord | |||
| 51 | + | } | |||
| 52 | + | cwdLen := int(cwdLen32) | |||
| 53 | + | if len(value) != 4+cwdLen+8+8 { | |||
| 49 | return Record{}, errMalformedRecord | 54 | return Record{}, errMalformedRecord | ||
| 50 | } | 55 | } | ||
| 51 | off := 4 + cwdLen | 56 | off := 4 + cwdLen | ||
M internal/ports/ports.go +1 -0
| @@ -50,6 +50,7 @@ type Store interface { | |||
| 50 | 50 | Get(key []byte) ([]byte, bool) | |
| 51 | 51 | Set(key, val []byte) error | |
| 52 | 52 | Delete(key []byte) error | |
| 53 | + | // Range iterates key/value pairs; fn returning false stops iteration early. | |
| 53 | 54 | Range(fn func(k, v []byte) bool) | |
| 54 | 55 | Sync() error | |
| 55 | 56 | Close() error | |
| @@ -50,6 +50,7 @@ type Store interface { | |||||
| 50 | Get(key []byte) ([]byte, bool) | 50 | Get(key []byte) ([]byte, bool) | ||
| 51 | Set(key, val []byte) error | 51 | Set(key, val []byte) error | ||
| 52 | Delete(key []byte) error | 52 | Delete(key []byte) error | ||
| 53 | + | // Range iterates key/value pairs; fn returning false stops iteration early. | |||
| 53 | Range(fn func(k, v []byte) bool) | 54 | Range(fn func(k, v []byte) bool) | ||
| 54 | Sync() error | 55 | Sync() error | ||
| 55 | Close() error | 56 | Close() error | ||
M internal/usecase/daemon/picker.go +1 -1
| @@ -91,7 +91,7 @@ func (d *Daemon) pickerViews(cur *session) ([]picker.SessionView, int) { | |||
| 91 | 91 | views = append(views, picker.SessionView{ | |
| 92 | 92 | ID: domain.SessionID("stopped:" + s.name), | |
| 93 | 93 | Name: s.name, | |
| 94 | - | Tabs: []string{"(stopped)"}, | |
| 94 | + | Tabs: []string{""}, | |
| 95 | 95 | Stopped: true, | |
| 96 | 96 | }) | |
| 97 | 97 | } | |
| @@ -91,7 +91,7 @@ func (d *Daemon) pickerViews(cur *session) ([]picker.SessionView, int) { | |||||
| 91 | views = append(views, picker.SessionView{ | 91 | views = append(views, picker.SessionView{ | ||
| 92 | ID: domain.SessionID("stopped:" + s.name), | 92 | ID: domain.SessionID("stopped:" + s.name), | ||
| 93 | Name: s.name, | 93 | Name: s.name, | ||
| 94 | - | Tabs: []string{"(stopped)"}, | 94 | + | Tabs: []string{""}, |
| 95 | Stopped: true, | 95 | Stopped: true, | ||
| 96 | }) | 96 | }) | ||
| 97 | } | 97 | } | ||
M internal/usecase/picker/model_test.go +1 -1
| @@ -102,7 +102,7 @@ func TestSelectedMapping(t *testing.T) { | |||
| 102 | 102 | } | |
| 103 | 103 | ||
| 104 | 104 | func TestStoppedSessionSelectableAndRendered(t *testing.T) { | |
| 105 | - | m := New([]SessionView{{ID: "stopped:work", Name: "work", Tabs: []string{"(stopped)"}, Stopped: true}}, "", 0) | |
| 105 | + | m := New([]SessionView{{ID: "stopped:work", Name: "work", Tabs: []string{""}, Stopped: true}}, "", 0) | |
| 106 | 106 | got, ok := m.Selected() | |
| 107 | 107 | require.True(t, ok) | |
| 108 | 108 | require.Equal(t, Target{Session: "stopped:work", Name: "work", TabIndex: 0, Stopped: true}, got) | |
| @@ -102,7 +102,7 @@ func TestSelectedMapping(t *testing.T) { | |||||
| 102 | } | 102 | } | ||
| 103 | 103 | ||||
| 104 | func TestStoppedSessionSelectableAndRendered(t *testing.T) { | 104 | func TestStoppedSessionSelectableAndRendered(t *testing.T) { | ||
| 105 | - | m := New([]SessionView{{ID: "stopped:work", Name: "work", Tabs: []string{"(stopped)"}, Stopped: true}}, "", 0) | 105 | + | m := New([]SessionView{{ID: "stopped:work", Name: "work", Tabs: []string{""}, Stopped: true}}, "", 0) |
| 106 | got, ok := m.Selected() | 106 | got, ok := m.Selected() | ||
| 107 | require.True(t, ok) | 107 | require.True(t, ok) | ||
| 108 | require.Equal(t, Target{Session: "stopped:work", Name: "work", TabIndex: 0, Stopped: true}, got) | 108 | require.Equal(t, Target{Session: "stopped:work", Name: "work", TabIndex: 0, Stopped: true}, got) | ||
M pkg/kv/kv.go +13 -5
| @@ -3,6 +3,7 @@ package kv | |||
| 3 | 3 | import ( | |
| 4 | 4 | "encoding/binary" | |
| 5 | 5 | "errors" | |
| 6 | + | "fmt" | |
| 6 | 7 | "hash/crc32" | |
| 7 | 8 | "io" | |
| 8 | 9 | "os" | |
| @@ -203,12 +204,19 @@ func (s *Store) appendRecord(op byte, key, val []byte) error { | |||
| 203 | 204 | } | |
| 204 | 205 | n, err := s.file.Write(buf) | |
| 205 | 206 | if err != nil || n != len(buf) { | |
| 206 | - | _ = s.file.Truncate(pos) | |
| 207 | - | _, _ = s.file.Seek(pos, io.SeekStart) | |
| 208 | - | if err != nil { | |
| 209 | - | return err | |
| 207 | + | writeErr := err | |
| 208 | + | if writeErr == nil { | |
| 209 | + | writeErr = io.ErrShortWrite | |
| 210 | + | } | |
| 211 | + | if tErr := s.file.Truncate(pos); tErr != nil { | |
| 212 | + | s.closed = true | |
| 213 | + | return fmt.Errorf("write failed (%v) and recovery truncate failed: %w", writeErr, tErr) | |
| 214 | + | } | |
| 215 | + | if _, sErr := s.file.Seek(pos, io.SeekStart); sErr != nil { | |
| 216 | + | s.closed = true | |
| 217 | + | return fmt.Errorf("write failed (%v) and recovery seek failed: %w", writeErr, sErr) | |
| 210 | 218 | } | |
| 211 | - | return io.ErrShortWrite | |
| 219 | + | return writeErr | |
| 212 | 220 | } | |
| 213 | 221 | ||
| 214 | 222 | k := string(key) | |
| @@ -3,6 +3,7 @@ package kv | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "encoding/binary" | 4 | "encoding/binary" | ||
| 5 | "errors" | 5 | "errors" | ||
| 6 | + | "fmt" | |||
| 6 | "hash/crc32" | 7 | "hash/crc32" | ||
| 7 | "io" | 8 | "io" | ||
| 8 | "os" | 9 | "os" | ||
| @@ -203,12 +204,19 @@ func (s *Store) appendRecord(op byte, key, val []byte) error { | |||||
| 203 | } | 204 | } | ||
| 204 | n, err := s.file.Write(buf) | 205 | n, err := s.file.Write(buf) | ||
| 205 | if err != nil || n != len(buf) { | 206 | if err != nil || n != len(buf) { | ||
| 206 | - | _ = s.file.Truncate(pos) | 207 | + | writeErr := err |
| 207 | - | _, _ = s.file.Seek(pos, io.SeekStart) | 208 | + | if writeErr == nil { |
| 208 | - | if err != nil { | 209 | + | writeErr = io.ErrShortWrite |
| 209 | - | return err | 210 | + | } |
| 211 | + | if tErr := s.file.Truncate(pos); tErr != nil { | |||
| 212 | + | s.closed = true | |||
| 213 | + | return fmt.Errorf("write failed (%v) and recovery truncate failed: %w", writeErr, tErr) | |||
| 214 | + | } | |||
| 215 | + | if _, sErr := s.file.Seek(pos, io.SeekStart); sErr != nil { | |||
| 216 | + | s.closed = true | |||
| 217 | + | return fmt.Errorf("write failed (%v) and recovery seek failed: %w", writeErr, sErr) | |||
| 210 | } | 218 | } | ||
| 211 | - | return io.ErrShortWrite | 219 | + | return writeErr |
| 212 | } | 220 | } | ||
| 213 | 221 | ||||
| 214 | k := string(key) | 222 | k := string(key) | ||