detached 049957e

fix(snapshot): address coderabbit findings

commit
049957ef17e32211be90634fa7c4f25538e4f2aa
author
brice · · 2026-07-06 08:32 UTC
parent
caf28f0
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
10 files · +187 -100
Unified
UnifiedSplit
M internal/adapters/config/config.go +8 -5
@@ -71,7 +71,9 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) {
7171 }
7272 cfg.Codes[codeKey] = value
7373 case key == "snapshot.restore_processes":
74- cfg.Snapshot.RestoreProcesses = parseProcessList(value, lineNo, &warnings)
74+ var processWarnings []domain.Warning
75+ cfg.Snapshot.RestoreProcesses, processWarnings = parseProcessList(value, lineNo)
76+ warnings = append(warnings, processWarnings...)
7577 cfg.Snapshot.RestoreProcessesSet = true
7678 default:
7779 if seenBindingKeys[key] {
@@ -170,12 +172,13 @@ func updateBindingEntry(entries []domain.ConfigEntry, key, value string) {
170172 }
171173 }
172174
173-func parseProcessList(value string, lineNo int, warnings *[]domain.Warning) []string {
175+func parseProcessList(value string, lineNo int) ([]string, []domain.Warning) {
174176 if strings.TrimSpace(value) == "" {
175- return nil
177+ return nil, nil
176178 }
177179 parts := strings.Split(value, ",")
178180 out := make([]string, 0, len(parts))
181+ warnings := make([]domain.Warning, 0)
179182 seen := make(map[string]struct{}, len(parts))
180183 for _, part := range parts {
181184 item := strings.TrimSpace(part)
@@ -183,7 +186,7 @@ func parseProcessList(value string, lineNo int, warnings *[]domain.Warning) []st
183186 continue
184187 }
185188 if !processNamePattern.MatchString(item) {
186- *warnings = append(*warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid snapshot restore process %q", item)})
189+ warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid snapshot restore process %q", item)})
187190 continue
188191 }
189192 if _, ok := seen[item]; ok {
@@ -192,7 +195,7 @@ func parseProcessList(value string, lineNo int, warnings *[]domain.Warning) []st
192195 seen[item] = struct{}{}
193196 out = append(out, item)
194197 }
195- return out
198+ return out, warnings
196199 }
197200
198201 func parseTheme(value string) (domain.ThemeMode, bool) {
@@ -71,7 +71,9 @@ func Parse(r io.Reader) (domain.Config, []domain.Warning, error) {
71 }71 }
72 cfg.Codes[codeKey] = value72 cfg.Codes[codeKey] = value
73 case key == "snapshot.restore_processes":73 case key == "snapshot.restore_processes":
74- cfg.Snapshot.RestoreProcesses = parseProcessList(value, lineNo, &warnings)74+ var processWarnings []domain.Warning
75+ cfg.Snapshot.RestoreProcesses, processWarnings = parseProcessList(value, lineNo)
76+ warnings = append(warnings, processWarnings...)
75 cfg.Snapshot.RestoreProcessesSet = true77 cfg.Snapshot.RestoreProcessesSet = true
76 default:78 default:
77 if seenBindingKeys[key] {79 if seenBindingKeys[key] {
@@ -170,12 +172,13 @@ func updateBindingEntry(entries []domain.ConfigEntry, key, value string) {
170 }172 }
171 }173 }
172 174
173-func parseProcessList(value string, lineNo int, warnings *[]domain.Warning) []string {175+func parseProcessList(value string, lineNo int) ([]string, []domain.Warning) {
174 if strings.TrimSpace(value) == "" {176 if strings.TrimSpace(value) == "" {
175- return nil177+ return nil, nil
176 }178 }
177 parts := strings.Split(value, ",")179 parts := strings.Split(value, ",")
178 out := make([]string, 0, len(parts))180 out := make([]string, 0, len(parts))
181+ warnings := make([]domain.Warning, 0)
179 seen := make(map[string]struct{}, len(parts))182 seen := make(map[string]struct{}, len(parts))
180 for _, part := range parts {183 for _, part := range parts {
181 item := strings.TrimSpace(part)184 item := strings.TrimSpace(part)
@@ -183,7 +186,7 @@ func parseProcessList(value string, lineNo int, warnings *[]domain.Warning) []st
183 continue186 continue
184 }187 }
185 if !processNamePattern.MatchString(item) {188 if !processNamePattern.MatchString(item) {
186- *warnings = append(*warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid snapshot restore process %q", item)})189+ warnings = append(warnings, domain.Warning{Line: lineNo, Msg: fmt.Sprintf("invalid snapshot restore process %q", item)})
187 continue190 continue
188 }191 }
189 if _, ok := seen[item]; ok {192 if _, ok := seen[item]; ok {
@@ -192,7 +195,7 @@ func parseProcessList(value string, lineNo int, warnings *[]domain.Warning) []st
192 seen[item] = struct{}{}195 seen[item] = struct{}{}
193 out = append(out, item)196 out = append(out, item)
194 }197 }
195- return out198+ return out, warnings
196 }199 }
197 200
198 func parseTheme(value string) (domain.ThemeMode, bool) {201 func parseTheme(value string) (domain.ThemeMode, bool) {
M internal/adapters/config/config_test.go +44 -35
@@ -91,6 +91,49 @@ func TestParse(t *testing.T) {
9191 {Line: 3, Msg: "missing key"},
9292 },
9393 },
94+ {
95+ name: "snapshot restore processes",
96+ input: "snapshot.restore_processes = claude, codex, pi, opencode, btop\n",
97+ want: domain.Config{
98+ Theme: domain.ThemeAuto,
99+ BindingEntries: []domain.ConfigEntry{},
100+ Codes: map[string]string{},
101+ Snapshot: domain.SnapshotConfig{
102+ RestoreProcesses: []string{"claude", "codex", "pi", "opencode", "btop"},
103+ RestoreProcessesSet: true,
104+ },
105+ },
106+ },
107+ {
108+ name: "snapshot restore processes empty disables",
109+ input: "snapshot.restore_processes =\n",
110+ want: domain.Config{
111+ Theme: domain.ThemeAuto,
112+ BindingEntries: []domain.ConfigEntry{},
113+ Codes: map[string]string{},
114+ Snapshot: domain.SnapshotConfig{
115+ RestoreProcessesSet: true,
116+ },
117+ },
118+ },
119+ {
120+ name: "snapshot restore processes warns on malformed entries",
121+ input: "snapshot.restore_processes = claude, custom-tool, /bin/sh, :all:, bad name, claude, , zsh\n",
122+ want: domain.Config{
123+ Theme: domain.ThemeAuto,
124+ BindingEntries: []domain.ConfigEntry{},
125+ Codes: map[string]string{},
126+ Snapshot: domain.SnapshotConfig{
127+ RestoreProcesses: []string{"claude", "custom-tool", "zsh"},
128+ RestoreProcessesSet: true,
129+ },
130+ },
131+ wantWarnings: []domain.Warning{
132+ {Line: 1, Msg: "invalid snapshot restore process \"/bin/sh\""},
133+ {Line: 1, Msg: "invalid snapshot restore process \":all:\""},
134+ {Line: 1, Msg: "invalid snapshot restore process \"bad name\""},
135+ },
136+ },
94137 }
95138
96139 for _, tt := range tests {
@@ -98,7 +141,7 @@ func TestParse(t *testing.T) {
98141 t.Parallel()
99142
100143 if tt.want.Snapshot.RestoreProcesses == nil && !tt.want.Snapshot.RestoreProcessesSet {
101- tt.want.Snapshot.RestoreProcesses = append([]string(nil), domain.DefaultSnapshotRestoreProcesses...)
144+ tt.want.Snapshot.RestoreProcesses = append([]string(nil), domain.DefaultSnapshotRestoreProcesses()...)
102145 }
103146 got, warnings, err := Parse(strings.NewReader(tt.input))
104147 if err != nil {
@@ -114,40 +157,6 @@ func TestParse(t *testing.T) {
114157 }
115158 }
116159
117-func TestParseSnapshotRestoreProcesses(t *testing.T) {
118- t.Parallel()
119-
120- cfg, warnings, err := Parse(strings.NewReader("snapshot.restore_processes = claude, codex, pi, opencode, btop\n"))
121- require.NoError(t, err)
122- require.Empty(t, warnings)
123- require.Equal(t, []string{"claude", "codex", "pi", "opencode", "btop"}, cfg.Snapshot.RestoreProcesses)
124- require.True(t, cfg.Snapshot.RestoreProcessesSet)
125-}
126-
127-func TestParseSnapshotRestoreProcessesEmptyDisables(t *testing.T) {
128- t.Parallel()
129-
130- cfg, warnings, err := Parse(strings.NewReader("snapshot.restore_processes =\n"))
131- require.NoError(t, err)
132- require.Empty(t, warnings)
133- require.Empty(t, cfg.Snapshot.RestoreProcesses)
134- require.True(t, cfg.Snapshot.RestoreProcessesSet)
135-}
136-
137-func TestParseSnapshotRestoreProcessesWarnsOnMalformedEntries(t *testing.T) {
138- t.Parallel()
139-
140- cfg, warnings, err := Parse(strings.NewReader("snapshot.restore_processes = claude, custom-tool, /bin/sh, :all:, bad name, claude, , zsh\n"))
141- require.NoError(t, err)
142- require.Equal(t, []domain.Warning{
143- {Line: 1, Msg: "invalid snapshot restore process \"/bin/sh\""},
144- {Line: 1, Msg: "invalid snapshot restore process \":all:\""},
145- {Line: 1, Msg: "invalid snapshot restore process \"bad name\""},
146- }, warnings)
147- require.Equal(t, []string{"claude", "custom-tool", "zsh"}, cfg.Snapshot.RestoreProcesses)
148- require.True(t, cfg.Snapshot.RestoreProcessesSet)
149-}
150-
151160 func TestDefaultsCopiesSnapshotRestoreProcesses(t *testing.T) {
152161 t.Parallel()
153162
@@ -91,6 +91,49 @@ func TestParse(t *testing.T) {
91 {Line: 3, Msg: "missing key"},91 {Line: 3, Msg: "missing key"},
92 },92 },
93 },93 },
94+ {
95+ name: "snapshot restore processes",
96+ input: "snapshot.restore_processes = claude, codex, pi, opencode, btop\n",
97+ want: domain.Config{
98+ Theme: domain.ThemeAuto,
99+ BindingEntries: []domain.ConfigEntry{},
100+ Codes: map[string]string{},
101+ Snapshot: domain.SnapshotConfig{
102+ RestoreProcesses: []string{"claude", "codex", "pi", "opencode", "btop"},
103+ RestoreProcessesSet: true,
104+ },
105+ },
106+ },
107+ {
108+ name: "snapshot restore processes empty disables",
109+ input: "snapshot.restore_processes =\n",
110+ want: domain.Config{
111+ Theme: domain.ThemeAuto,
112+ BindingEntries: []domain.ConfigEntry{},
113+ Codes: map[string]string{},
114+ Snapshot: domain.SnapshotConfig{
115+ RestoreProcessesSet: true,
116+ },
117+ },
118+ },
119+ {
120+ name: "snapshot restore processes warns on malformed entries",
121+ input: "snapshot.restore_processes = claude, custom-tool, /bin/sh, :all:, bad name, claude, , zsh\n",
122+ want: domain.Config{
123+ Theme: domain.ThemeAuto,
124+ BindingEntries: []domain.ConfigEntry{},
125+ Codes: map[string]string{},
126+ Snapshot: domain.SnapshotConfig{
127+ RestoreProcesses: []string{"claude", "custom-tool", "zsh"},
128+ RestoreProcessesSet: true,
129+ },
130+ },
131+ wantWarnings: []domain.Warning{
132+ {Line: 1, Msg: "invalid snapshot restore process \"/bin/sh\""},
133+ {Line: 1, Msg: "invalid snapshot restore process \":all:\""},
134+ {Line: 1, Msg: "invalid snapshot restore process \"bad name\""},
135+ },
136+ },
94 }137 }
95 138
96 for _, tt := range tests {139 for _, tt := range tests {
@@ -98,7 +141,7 @@ func TestParse(t *testing.T) {
98 t.Parallel()141 t.Parallel()
99 142
100 if tt.want.Snapshot.RestoreProcesses == nil && !tt.want.Snapshot.RestoreProcessesSet {143 if tt.want.Snapshot.RestoreProcesses == nil && !tt.want.Snapshot.RestoreProcessesSet {
101- tt.want.Snapshot.RestoreProcesses = append([]string(nil), domain.DefaultSnapshotRestoreProcesses...)144+ tt.want.Snapshot.RestoreProcesses = append([]string(nil), domain.DefaultSnapshotRestoreProcesses()...)
102 }145 }
103 got, warnings, err := Parse(strings.NewReader(tt.input))146 got, warnings, err := Parse(strings.NewReader(tt.input))
104 if err != nil {147 if err != nil {
@@ -114,40 +157,6 @@ func TestParse(t *testing.T) {
114 }157 }
115 }158 }
116 159
117-func TestParseSnapshotRestoreProcesses(t *testing.T) {
118- t.Parallel()
119-
120- cfg, warnings, err := Parse(strings.NewReader("snapshot.restore_processes = claude, codex, pi, opencode, btop\n"))
121- require.NoError(t, err)
122- require.Empty(t, warnings)
123- require.Equal(t, []string{"claude", "codex", "pi", "opencode", "btop"}, cfg.Snapshot.RestoreProcesses)
124- require.True(t, cfg.Snapshot.RestoreProcessesSet)
125-}
126-
127-func TestParseSnapshotRestoreProcessesEmptyDisables(t *testing.T) {
128- t.Parallel()
129-
130- cfg, warnings, err := Parse(strings.NewReader("snapshot.restore_processes =\n"))
131- require.NoError(t, err)
132- require.Empty(t, warnings)
133- require.Empty(t, cfg.Snapshot.RestoreProcesses)
134- require.True(t, cfg.Snapshot.RestoreProcessesSet)
135-}
136-
137-func TestParseSnapshotRestoreProcessesWarnsOnMalformedEntries(t *testing.T) {
138- t.Parallel()
139-
140- cfg, warnings, err := Parse(strings.NewReader("snapshot.restore_processes = claude, custom-tool, /bin/sh, :all:, bad name, claude, , zsh\n"))
141- require.NoError(t, err)
142- require.Equal(t, []domain.Warning{
143- {Line: 1, Msg: "invalid snapshot restore process \"/bin/sh\""},
144- {Line: 1, Msg: "invalid snapshot restore process \":all:\""},
145- {Line: 1, Msg: "invalid snapshot restore process \"bad name\""},
146- }, warnings)
147- require.Equal(t, []string{"claude", "custom-tool", "zsh"}, cfg.Snapshot.RestoreProcesses)
148- require.True(t, cfg.Snapshot.RestoreProcessesSet)
149-}
150-
151 func TestDefaultsCopiesSnapshotRestoreProcesses(t *testing.T) {160 func TestDefaultsCopiesSnapshotRestoreProcesses(t *testing.T) {
152 t.Parallel()161 t.Parallel()
153 162
M internal/domain/config.go +8 -3
@@ -24,12 +24,17 @@ type SnapshotConfig struct {
2424 RestoreProcessesSet bool
2525 }
2626
27-// DefaultSnapshotRestoreProcesses is the default allowlist for process restore.
28-var DefaultSnapshotRestoreProcesses = []string{
27+// defaultSnapshotRestoreProcesses is the default allowlist for process restore.
28+var defaultSnapshotRestoreProcesses = []string{
2929 "vi", "vim", "nvim", "emacs", "man", "less", "more", "tail", "top", "htop", "btop",
3030 "claude", "codex", "pi", "opencode",
3131 }
3232
33+// DefaultSnapshotRestoreProcesses returns a copy of the default process restore allowlist.
34+func DefaultSnapshotRestoreProcesses() []string {
35+ return append([]string(nil), defaultSnapshotRestoreProcesses...)
36+}
37+
3338 // Config is the user-editable vev configuration after parsing. Unknown binding
3439 // keys are preserved here (in BindingEntries, in file order) so the usecase
3540 // layer can decide which actions it understands.
@@ -54,7 +59,7 @@ func Defaults() Config {
5459 BindingEntries: []ConfigEntry{},
5560 Codes: map[string]string{},
5661 Snapshot: SnapshotConfig{
57- RestoreProcesses: append([]string(nil), DefaultSnapshotRestoreProcesses...),
62+ RestoreProcesses: DefaultSnapshotRestoreProcesses(),
5863 },
5964 }
6065 }
@@ -24,12 +24,17 @@ type SnapshotConfig struct {
24 RestoreProcessesSet bool24 RestoreProcessesSet bool
25 }25 }
26 26
27-// DefaultSnapshotRestoreProcesses is the default allowlist for process restore.27+// defaultSnapshotRestoreProcesses is the default allowlist for process restore.
28-var DefaultSnapshotRestoreProcesses = []string{28+var defaultSnapshotRestoreProcesses = []string{
29 "vi", "vim", "nvim", "emacs", "man", "less", "more", "tail", "top", "htop", "btop",29 "vi", "vim", "nvim", "emacs", "man", "less", "more", "tail", "top", "htop", "btop",
30 "claude", "codex", "pi", "opencode",30 "claude", "codex", "pi", "opencode",
31 }31 }
32 32
33+// DefaultSnapshotRestoreProcesses returns a copy of the default process restore allowlist.
34+func DefaultSnapshotRestoreProcesses() []string {
35+ return append([]string(nil), defaultSnapshotRestoreProcesses...)
36+}
37+
33 // Config is the user-editable vev configuration after parsing. Unknown binding38 // Config is the user-editable vev configuration after parsing. Unknown binding
34 // keys are preserved here (in BindingEntries, in file order) so the usecase39 // keys are preserved here (in BindingEntries, in file order) so the usecase
35 // layer can decide which actions it understands.40 // layer can decide which actions it understands.
@@ -54,7 +59,7 @@ func Defaults() Config {
54 BindingEntries: []ConfigEntry{},59 BindingEntries: []ConfigEntry{},
55 Codes: map[string]string{},60 Codes: map[string]string{},
56 Snapshot: SnapshotConfig{61 Snapshot: SnapshotConfig{
57- RestoreProcesses: append([]string(nil), DefaultSnapshotRestoreProcesses...),62+ RestoreProcesses: DefaultSnapshotRestoreProcesses(),
58 },63 },
59 }64 }
60 }65 }
M internal/platform/process_linux.go +3 -8
@@ -35,14 +35,9 @@ func processArgv(path string, pid int) ([]string, error) {
3535 return nil, fmt.Errorf("process argv: empty argv for pid %d", pid)
3636 }
3737 parts := bytes.Split(data, []byte{0})
38- argv := make([]string, 0, len(parts))
39- for _, part := range parts {
40- if len(part) > 0 {
41- argv = append(argv, string(part))
42- }
43- }
44- if len(argv) == 0 {
45- return nil, fmt.Errorf("process argv: empty argv for pid %d", pid)
38+ argv := make([]string, len(parts))
39+ for i, part := range parts {
40+ argv[i] = string(part)
4641 }
4742 return argv, nil
4843 }
@@ -35,14 +35,9 @@ func processArgv(path string, pid int) ([]string, error) {
35 return nil, fmt.Errorf("process argv: empty argv for pid %d", pid)35 return nil, fmt.Errorf("process argv: empty argv for pid %d", pid)
36 }36 }
37 parts := bytes.Split(data, []byte{0})37 parts := bytes.Split(data, []byte{0})
38- argv := make([]string, 0, len(parts))38+ argv := make([]string, len(parts))
39- for _, part := range parts {39+ for i, part := range parts {
40- if len(part) > 0 {40+ argv[i] = string(part)
41- argv = append(argv, string(part))
42- }
43- }
44- if len(argv) == 0 {
45- return nil, fmt.Errorf("process argv: empty argv for pid %d", pid)
46 }41 }
47 return argv, nil42 return argv, nil
48 }43 }
M internal/platform/process_linux_test.go +62 -21
@@ -27,19 +27,40 @@ func TestProcessArgvRejectsEmptyCmdline(t *testing.T) {
2727 }
2828 }
2929
30-func TestProcessGroupArgvSelection(t *testing.T) {
31- recs := []processRecord{{pid: 10, pgrp: 7}, {pid: 12, pgrp: 7}, {pid: 20, pgrp: 9}}
32- pid, ok := selectProcessGroupPID(recs, 7, 10)
33- if !ok || pid != 12 {
34- t.Fatalf("selectProcessGroupPID = %d, %v; want 12, true", pid, ok)
30+func TestSelectProcessGroupPID(t *testing.T) {
31+ tests := []struct {
32+ name string
33+ recs []processRecord
34+ pgid int
35+ shell int
36+ wantPID int
37+ wantOK bool
38+ }{
39+ {
40+ name: "selects other member",
41+ recs: []processRecord{{pid: 10, pgrp: 7}, {pid: 12, pgrp: 7}, {pid: 20, pgrp: 9}},
42+ pgid: 7,
43+ shell: 10,
44+ wantPID: 12,
45+ wantOK: true,
46+ },
47+ {
48+ name: "bare shell has no candidate",
49+ recs: []processRecord{{pid: 10, pgrp: 7}, {pid: 20, pgrp: 9}},
50+ pgid: 7,
51+ shell: 10,
52+ wantPID: 0,
53+ wantOK: false,
54+ },
3555 }
36-}
3756
38-func TestProcessGroupArgvBareShellReturnsNoPID(t *testing.T) {
39- recs := []processRecord{{pid: 10, pgrp: 7}, {pid: 20, pgrp: 9}}
40- pid, ok := selectProcessGroupPID(recs, 7, 10)
41- if ok || pid != 0 {
42- t.Fatalf("selectProcessGroupPID bare shell = %d, %v; want 0, false", pid, ok)
57+ for _, tt := range tests {
58+ t.Run(tt.name, func(t *testing.T) {
59+ pid, ok := selectProcessGroupPID(tt.recs, tt.pgid, tt.shell)
60+ if ok != tt.wantOK || pid != tt.wantPID {
61+ t.Fatalf("selectProcessGroupPID = %d, %v; want %d, %v", pid, ok, tt.wantPID, tt.wantOK)
62+ }
63+ })
4364 }
4465 }
4566
@@ -51,16 +72,36 @@ func TestParseStatPgrp(t *testing.T) {
5172 }
5273
5374 func TestProcessArgvFileParsing(t *testing.T) {
54- path := filepath.Join(t.TempDir(), "cmdline")
55- if err := os.WriteFile(path, []byte("cmd\x00two words\x00"), 0o600); err != nil {
56- t.Fatal(err)
75+ tests := []struct {
76+ name string
77+ data []byte
78+ want []string
79+ }{
80+ {
81+ name: "splits nul-delimited argv",
82+ data: []byte("cmd\x00two words\x00"),
83+ want: []string{"cmd", "two words"},
84+ },
85+ {
86+ name: "preserves empty argument",
87+ data: []byte("cmd\x00\x00arg\x00"),
88+ want: []string{"cmd", "", "arg"},
89+ },
5790 }
58- got, err := processArgv(path, 123)
59- if err != nil {
60- t.Fatal(err)
61- }
62- want := []string{"cmd", "two words"}
63- if !reflect.DeepEqual(got, want) {
64- t.Fatalf("processArgv = %#v; want %#v", got, want)
91+
92+ for _, tt := range tests {
93+ t.Run(tt.name, func(t *testing.T) {
94+ path := filepath.Join(t.TempDir(), "cmdline")
95+ if err := os.WriteFile(path, tt.data, 0o600); err != nil {
96+ t.Fatal(err)
97+ }
98+ got, err := processArgv(path, 123)
99+ if err != nil {
100+ t.Fatal(err)
101+ }
102+ if !reflect.DeepEqual(got, tt.want) {
103+ t.Fatalf("processArgv = %#v; want %#v", got, tt.want)
104+ }
105+ })
65106 }
66107 }
@@ -27,19 +27,40 @@ func TestProcessArgvRejectsEmptyCmdline(t *testing.T) {
27 }27 }
28 }28 }
29 29
30-func TestProcessGroupArgvSelection(t *testing.T) {30+func TestSelectProcessGroupPID(t *testing.T) {
31- recs := []processRecord{{pid: 10, pgrp: 7}, {pid: 12, pgrp: 7}, {pid: 20, pgrp: 9}}31+ tests := []struct {
32- pid, ok := selectProcessGroupPID(recs, 7, 10)32+ name string
33- if !ok || pid != 12 {33+ recs []processRecord
34- t.Fatalf("selectProcessGroupPID = %d, %v; want 12, true", pid, ok)34+ pgid int
35+ shell int
36+ wantPID int
37+ wantOK bool
38+ }{
39+ {
40+ name: "selects other member",
41+ recs: []processRecord{{pid: 10, pgrp: 7}, {pid: 12, pgrp: 7}, {pid: 20, pgrp: 9}},
42+ pgid: 7,
43+ shell: 10,
44+ wantPID: 12,
45+ wantOK: true,
46+ },
47+ {
48+ name: "bare shell has no candidate",
49+ recs: []processRecord{{pid: 10, pgrp: 7}, {pid: 20, pgrp: 9}},
50+ pgid: 7,
51+ shell: 10,
52+ wantPID: 0,
53+ wantOK: false,
54+ },
35 }55 }
36-}
37 56
38-func TestProcessGroupArgvBareShellReturnsNoPID(t *testing.T) {57+ for _, tt := range tests {
39- recs := []processRecord{{pid: 10, pgrp: 7}, {pid: 20, pgrp: 9}}58+ t.Run(tt.name, func(t *testing.T) {
40- pid, ok := selectProcessGroupPID(recs, 7, 10)59+ pid, ok := selectProcessGroupPID(tt.recs, tt.pgid, tt.shell)
41- if ok || pid != 0 {60+ if ok != tt.wantOK || pid != tt.wantPID {
42- t.Fatalf("selectProcessGroupPID bare shell = %d, %v; want 0, false", pid, ok)61+ t.Fatalf("selectProcessGroupPID = %d, %v; want %d, %v", pid, ok, tt.wantPID, tt.wantOK)
62+ }
63+ })
43 }64 }
44 }65 }
45 66
@@ -51,16 +72,36 @@ func TestParseStatPgrp(t *testing.T) {
51 }72 }
52 73
53 func TestProcessArgvFileParsing(t *testing.T) {74 func TestProcessArgvFileParsing(t *testing.T) {
54- path := filepath.Join(t.TempDir(), "cmdline")75+ tests := []struct {
55- if err := os.WriteFile(path, []byte("cmd\x00two words\x00"), 0o600); err != nil {76+ name string
56- t.Fatal(err)77+ data []byte
78+ want []string
79+ }{
80+ {
81+ name: "splits nul-delimited argv",
82+ data: []byte("cmd\x00two words\x00"),
83+ want: []string{"cmd", "two words"},
84+ },
85+ {
86+ name: "preserves empty argument",
87+ data: []byte("cmd\x00\x00arg\x00"),
88+ want: []string{"cmd", "", "arg"},
89+ },
57 }90 }
58- got, err := processArgv(path, 123)91+
59- if err != nil {92+ for _, tt := range tests {
60- t.Fatal(err)93+ t.Run(tt.name, func(t *testing.T) {
61- }94+ path := filepath.Join(t.TempDir(), "cmdline")
62- want := []string{"cmd", "two words"}95+ if err := os.WriteFile(path, tt.data, 0o600); err != nil {
63- if !reflect.DeepEqual(got, want) {96+ t.Fatal(err)
64- t.Fatalf("processArgv = %#v; want %#v", got, want)97+ }
98+ got, err := processArgv(path, 123)
99+ if err != nil {
100+ t.Fatal(err)
101+ }
102+ if !reflect.DeepEqual(got, tt.want) {
103+ t.Fatalf("processArgv = %#v; want %#v", got, tt.want)
104+ }
105+ })
65 }106 }
66 }107 }
M internal/ports/wire.go +6 -5
@@ -40,11 +40,12 @@ const (
4040
4141 // ErrorMsg codes.
4242 const (
43- ErrVersionMismatch uint16 = 1
44- ErrNoSuchSession uint16 = 2
45- ErrNameTaken uint16 = 3
46- ErrServerShutdown uint16 = 4
47- ErrInternal uint16 = 255
43+ ErrVersionMismatch uint16 = 1
44+ ErrNoSuchSession uint16 = 2
45+ ErrNameTaken uint16 = 3
46+ ErrServerShutdown uint16 = 4
47+ ErrInvalidSessionName uint16 = 5
48+ ErrInternal uint16 = 255
4849 )
4950
5051 // Detached reasons.
@@ -40,11 +40,12 @@ const (
40 40
41 // ErrorMsg codes.41 // ErrorMsg codes.
42 const (42 const (
43- ErrVersionMismatch uint16 = 143+ ErrVersionMismatch uint16 = 1
44- ErrNoSuchSession uint16 = 244+ ErrNoSuchSession uint16 = 2
45- ErrNameTaken uint16 = 345+ ErrNameTaken uint16 = 3
46- ErrServerShutdown uint16 = 446+ ErrServerShutdown uint16 = 4
47- ErrInternal uint16 = 25547+ ErrInvalidSessionName uint16 = 5
48+ ErrInternal uint16 = 255
48 )49 )
49 50
50 // Detached reasons.51 // Detached reasons.
M internal/ports/wire_test.go +8 -6
@@ -542,13 +542,15 @@ func TestMsgTypeConstantsDistinct(t *testing.T) {
542542
543543 t.Run("error codes", func(t *testing.T) {
544544 vals := map[uint16]string{
545- ErrVersionMismatch: "ErrVersionMismatch",
546- ErrNoSuchSession: "ErrNoSuchSession",
547- ErrNameTaken: "ErrNameTaken",
548- ErrInternal: "ErrInternal",
545+ ErrVersionMismatch: "ErrVersionMismatch",
546+ ErrNoSuchSession: "ErrNoSuchSession",
547+ ErrNameTaken: "ErrNameTaken",
548+ ErrServerShutdown: "ErrServerShutdown",
549+ ErrInvalidSessionName: "ErrInvalidSessionName",
550+ ErrInternal: "ErrInternal",
549551 }
550- if len(vals) != 4 {
551- t.Fatalf("expected 4 distinct error codes, got %d", len(vals))
552+ if len(vals) != 6 {
553+ t.Fatalf("expected 6 distinct error codes, got %d", len(vals))
552554 }
553555 })
554556
@@ -542,13 +542,15 @@ func TestMsgTypeConstantsDistinct(t *testing.T) {
542 542
543 t.Run("error codes", func(t *testing.T) {543 t.Run("error codes", func(t *testing.T) {
544 vals := map[uint16]string{544 vals := map[uint16]string{
545- ErrVersionMismatch: "ErrVersionMismatch",545+ ErrVersionMismatch: "ErrVersionMismatch",
546- ErrNoSuchSession: "ErrNoSuchSession",546+ ErrNoSuchSession: "ErrNoSuchSession",
547- ErrNameTaken: "ErrNameTaken",547+ ErrNameTaken: "ErrNameTaken",
548- ErrInternal: "ErrInternal",548+ ErrServerShutdown: "ErrServerShutdown",
549+ ErrInvalidSessionName: "ErrInvalidSessionName",
550+ ErrInternal: "ErrInternal",
549 }551 }
550- if len(vals) != 4 {552+ if len(vals) != 6 {
551- t.Fatalf("expected 4 distinct error codes, got %d", len(vals))553+ t.Fatalf("expected 6 distinct error codes, got %d", len(vals))
552 }554 }
553 })555 })
554 556
M internal/usecase/daemon/config.go +2 -2
@@ -130,7 +130,7 @@ func restoreProcessAllowlistFromConfig(cfg domain.SnapshotConfig) map[string]str
130130 if len(cfg.RestoreProcesses) > 0 {
131131 return buildRestoreProcessAllowlist(cfg.RestoreProcesses)
132132 }
133- return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses)
133+ return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses())
134134 }
135135
136136 func buildRestoreProcessAllowlist(values []string) map[string]struct{} {
@@ -148,7 +148,7 @@ func buildRestoreProcessAllowlist(values []string) map[string]struct{} {
148148 func (d *Daemon) restoreProcessAllowlistSnapshot() map[string]struct{} {
149149 allow := d.restoreProcessAllowlist.Load()
150150 if allow == nil {
151- return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses)
151+ return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses())
152152 }
153153 return *allow
154154 }
@@ -130,7 +130,7 @@ func restoreProcessAllowlistFromConfig(cfg domain.SnapshotConfig) map[string]str
130 if len(cfg.RestoreProcesses) > 0 {130 if len(cfg.RestoreProcesses) > 0 {
131 return buildRestoreProcessAllowlist(cfg.RestoreProcesses)131 return buildRestoreProcessAllowlist(cfg.RestoreProcesses)
132 }132 }
133- return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses)133+ return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses())
134 }134 }
135 135
136 func buildRestoreProcessAllowlist(values []string) map[string]struct{} {136 func buildRestoreProcessAllowlist(values []string) map[string]struct{} {
@@ -148,7 +148,7 @@ func buildRestoreProcessAllowlist(values []string) map[string]struct{} {
148 func (d *Daemon) restoreProcessAllowlistSnapshot() map[string]struct{} {148 func (d *Daemon) restoreProcessAllowlistSnapshot() map[string]struct{} {
149 allow := d.restoreProcessAllowlist.Load()149 allow := d.restoreProcessAllowlist.Load()
150 if allow == nil {150 if allow == nil {
151- return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses)151+ return buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses())
152 }152 }
153 return *allow153 return *allow
154 }154 }
M internal/usecase/daemon/config_test.go +43 -12
@@ -50,19 +50,50 @@ func TestApplyConfigHotReloadSwapsBindingsAndCodes(t *testing.T) {
5050 }
5151
5252 func TestApplyConfigSnapshotRestoreProcesses(t *testing.T) {
53- d := newTestDaemon(t, nil, stubClock{})
54-
55- _, ok := d.restoreProcessAllowlistSnapshot()["claude"]
56- require.True(t, ok)
57-
58- d.ApplyConfig(domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true}})
59- require.Empty(t, d.restoreProcessAllowlistSnapshot())
53+ tests := []struct {
54+ name string
55+ cfg domain.Config
56+ check func(t *testing.T, allow map[string]struct{})
57+ }{
58+ {
59+ name: "default allowlist",
60+ check: func(t *testing.T, allow map[string]struct{}) {
61+ t.Helper()
62+ require.Contains(t, allow, "claude")
63+ },
64+ },
65+ {
66+ name: "explicit empty disables restore",
67+ cfg: domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true}},
68+ check: func(t *testing.T, allow map[string]struct{}) {
69+ t.Helper()
70+ require.Empty(t, allow)
71+ },
72+ },
73+ {
74+ name: "explicit values are trimmed",
75+ cfg: domain.Config{Snapshot: domain.SnapshotConfig{
76+ RestoreProcessesSet: true,
77+ RestoreProcesses: []string{"less", "", " pi "},
78+ }},
79+ check: func(t *testing.T, allow map[string]struct{}) {
80+ t.Helper()
81+ require.Contains(t, allow, "less")
82+ require.Contains(t, allow, "pi")
83+ require.NotContains(t, allow, "")
84+ },
85+ },
86+ }
6087
61- d.ApplyConfig(domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true, RestoreProcesses: []string{"less", "", " pi "}}})
62- allow := d.restoreProcessAllowlistSnapshot()
63- require.Contains(t, allow, "less")
64- require.Contains(t, allow, "pi")
65- require.NotContains(t, allow, "")
88+ for _, tt := range tests {
89+ t.Run(tt.name, func(t *testing.T) {
90+ d := newTestDaemon(t, nil, stubClock{})
91+ if tt.cfg.Snapshot.RestoreProcessesSet || len(tt.cfg.Snapshot.RestoreProcesses) > 0 {
92+ d.ApplyConfig(tt.cfg)
93+ }
94+ tt.check(t, d.restoreProcessAllowlistSnapshot())
95+ })
96+ }
6697 }
6798
6899 func TestPaletteCommandsApplyOverridesAndResolveByCode(t *testing.T) {
@@ -50,19 +50,50 @@ func TestApplyConfigHotReloadSwapsBindingsAndCodes(t *testing.T) {
50 }50 }
51 51
52 func TestApplyConfigSnapshotRestoreProcesses(t *testing.T) {52 func TestApplyConfigSnapshotRestoreProcesses(t *testing.T) {
53- d := newTestDaemon(t, nil, stubClock{})53+ tests := []struct {
54-54+ name string
55- _, ok := d.restoreProcessAllowlistSnapshot()["claude"]55+ cfg domain.Config
56- require.True(t, ok)56+ check func(t *testing.T, allow map[string]struct{})
57-57+ }{
58- d.ApplyConfig(domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true}})58+ {
59- require.Empty(t, d.restoreProcessAllowlistSnapshot())59+ name: "default allowlist",
60+ check: func(t *testing.T, allow map[string]struct{}) {
61+ t.Helper()
62+ require.Contains(t, allow, "claude")
63+ },
64+ },
65+ {
66+ name: "explicit empty disables restore",
67+ cfg: domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true}},
68+ check: func(t *testing.T, allow map[string]struct{}) {
69+ t.Helper()
70+ require.Empty(t, allow)
71+ },
72+ },
73+ {
74+ name: "explicit values are trimmed",
75+ cfg: domain.Config{Snapshot: domain.SnapshotConfig{
76+ RestoreProcessesSet: true,
77+ RestoreProcesses: []string{"less", "", " pi "},
78+ }},
79+ check: func(t *testing.T, allow map[string]struct{}) {
80+ t.Helper()
81+ require.Contains(t, allow, "less")
82+ require.Contains(t, allow, "pi")
83+ require.NotContains(t, allow, "")
84+ },
85+ },
86+ }
60 87
61- d.ApplyConfig(domain.Config{Snapshot: domain.SnapshotConfig{RestoreProcessesSet: true, RestoreProcesses: []string{"less", "", " pi "}}})88+ for _, tt := range tests {
62- allow := d.restoreProcessAllowlistSnapshot()89+ t.Run(tt.name, func(t *testing.T) {
63- require.Contains(t, allow, "less")90+ d := newTestDaemon(t, nil, stubClock{})
64- require.Contains(t, allow, "pi")91+ if tt.cfg.Snapshot.RestoreProcessesSet || len(tt.cfg.Snapshot.RestoreProcesses) > 0 {
65- require.NotContains(t, allow, "")92+ d.ApplyConfig(tt.cfg)
93+ }
94+ tt.check(t, d.restoreProcessAllowlistSnapshot())
95+ })
96+ }
66 }97 }
67 98
68 func TestPaletteCommandsApplyOverridesAndResolveByCode(t *testing.T) {99 func TestPaletteCommandsApplyOverridesAndResolveByCode(t *testing.T) {
M internal/usecase/daemon/daemon.go +3 -3
@@ -273,7 +273,7 @@ func New(ptys ports.PTYFactory, clock ports.Clock, log *slog.Logger, opts ...Opt
273273 d.codeOverrides.Store(&empty)
274274 }
275275 if d.restoreProcessAllowlist.Load() == nil {
276- allow := buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses)
276+ allow := buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses())
277277 d.restoreProcessAllowlist.Store(&allow)
278278 }
279279 if records, err := d.persist.LoadAll(); err != nil {
@@ -632,11 +632,11 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl
632632 case ports.IntentNew:
633633 if h.Name == "" {
634634 d.mu.Unlock()
635- return nil, nil, &protoErr{ports.ErrInternal, "empty session name"}
635+ return nil, nil, &protoErr{ports.ErrInvalidSessionName, "empty session name"}
636636 }
637637 if err := domain.ValidateSessionName(h.Name); err != nil {
638638 d.mu.Unlock()
639- return nil, nil, &protoErr{ports.ErrInternal, err.Error()}
639+ return nil, nil, &protoErr{ports.ErrInvalidSessionName, err.Error()}
640640 }
641641 if d.nameLiveOrStoppedLocked(h.Name) {
642642 d.mu.Unlock()
@@ -273,7 +273,7 @@ func New(ptys ports.PTYFactory, clock ports.Clock, log *slog.Logger, opts ...Opt
273 d.codeOverrides.Store(&empty)273 d.codeOverrides.Store(&empty)
274 }274 }
275 if d.restoreProcessAllowlist.Load() == nil {275 if d.restoreProcessAllowlist.Load() == nil {
276- allow := buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses)276+ allow := buildRestoreProcessAllowlist(domain.DefaultSnapshotRestoreProcesses())
277 d.restoreProcessAllowlist.Store(&allow)277 d.restoreProcessAllowlist.Store(&allow)
278 }278 }
279 if records, err := d.persist.LoadAll(); err != nil {279 if records, err := d.persist.LoadAll(); err != nil {
@@ -632,11 +632,11 @@ func (d *Daemon) route(h ports.Hello, tr ports.Transport) (*session, *attachedCl
632 case ports.IntentNew:632 case ports.IntentNew:
633 if h.Name == "" {633 if h.Name == "" {
634 d.mu.Unlock()634 d.mu.Unlock()
635- return nil, nil, &protoErr{ports.ErrInternal, "empty session name"}635+ return nil, nil, &protoErr{ports.ErrInvalidSessionName, "empty session name"}
636 }636 }
637 if err := domain.ValidateSessionName(h.Name); err != nil {637 if err := domain.ValidateSessionName(h.Name); err != nil {
638 d.mu.Unlock()638 d.mu.Unlock()
639- return nil, nil, &protoErr{ports.ErrInternal, err.Error()}639+ return nil, nil, &protoErr{ports.ErrInvalidSessionName, err.Error()}
640 }640 }
641 if d.nameLiveOrStoppedLocked(h.Name) {641 if d.nameLiveOrStoppedLocked(h.Name) {
642 d.mu.Unlock()642 d.mu.Unlock()

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