fix(kill): address review feedback
- commit
- 6489acff9605f1ffe0f9b690bbf5f6dd8b25e398
- author
- brice · · 2026-07-03 07:30 UTC
- parent
- cd05aa6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +18 -12
M README.md +2 -2
| @@ -19,8 +19,8 @@ vev attach <name> attach to an existing named session (alias: a) | |||
| 19 | 19 | vev attach user@host[:session] attach to a remote vev daemon over SSH | |
| 20 | 20 | vev ls list sessions | |
| 21 | 21 | vev kill <name> kill a named session | |
| 22 | - | vev kill --all kill all sessions | |
| 23 | - | vev kill --daemon kill the active vev daemon | |
| 22 | + | vev kill --all kill all sessions and stop the daemon | |
| 23 | + | vev kill --daemon stop the active vev daemon | |
| 24 | 24 | vev --help show help | |
| 25 | 25 | vev --version show version | |
| 26 | 26 | ``` | |
| @@ -19,8 +19,8 @@ vev attach <name> attach to an existing named session (alias: a) | |||||
| 19 | vev attach user@host[:session] attach to a remote vev daemon over SSH | 19 | vev attach user@host[:session] attach to a remote vev daemon over SSH | ||
| 20 | vev ls list sessions | 20 | vev ls list sessions | ||
| 21 | vev kill <name> kill a named session | 21 | vev kill <name> kill a named session | ||
| 22 | - | vev kill --all kill all sessions | 22 | + | vev kill --all kill all sessions and stop the daemon |
| 23 | - | vev kill --daemon kill the active vev daemon | 23 | + | vev kill --daemon stop the active vev daemon |
| 24 | vev --help show help | 24 | vev --help show help | ||
| 25 | vev --version show version | 25 | vev --version show version | ||
| 26 | ``` | 26 | ``` | ||
M internal/app/daemon_integration_test.go +1 -0
| @@ -197,6 +197,7 @@ func TestIntegration_KillAllShutsDownDaemon(t *testing.T) { | |||
| 197 | 197 | ||
| 198 | 198 | killTr, err := ipc.Dial(dir) | |
| 199 | 199 | require.NoError(t, err) | |
| 200 | + | defer func() { _ = killTr.Close() }() | |
| 200 | 201 | require.NoError(t, killTr.Send(ports.Frame{Type: ports.MsgKill, Payload: ports.MarshalKill(ports.Kill{All: true})})) | |
| 201 | 202 | ||
| 202 | 203 | _, err = killTr.Recv() | |
| @@ -197,6 +197,7 @@ func TestIntegration_KillAllShutsDownDaemon(t *testing.T) { | |||||
| 197 | 197 | ||||
| 198 | killTr, err := ipc.Dial(dir) | 198 | killTr, err := ipc.Dial(dir) | ||
| 199 | require.NoError(t, err) | 199 | require.NoError(t, err) | ||
| 200 | + | defer func() { _ = killTr.Close() }() | |||
| 200 | require.NoError(t, killTr.Send(ports.Frame{Type: ports.MsgKill, Payload: ports.MarshalKill(ports.Kill{All: true})})) | 201 | require.NoError(t, killTr.Send(ports.Frame{Type: ports.MsgKill, Payload: ports.MarshalKill(ports.Kill{All: true})})) | ||
| 201 | 202 | ||||
| 202 | _, err = killTr.Recv() | 203 | _, err = killTr.Recv() | ||
M internal/app/run.go +9 -3
| @@ -71,8 +71,8 @@ usage: | |||
| 71 | 71 | attach through SSH to a remote vev daemon | |
| 72 | 72 | vev ls list sessions | |
| 73 | 73 | vev kill <name> kill a named session | |
| 74 | - | vev kill --all kill all sessions | |
| 75 | - | vev kill --daemon kill the active vev daemon | |
| 74 | + | vev kill --all kill all sessions and stop the daemon | |
| 75 | + | vev kill --daemon stop the active vev daemon | |
| 76 | 76 | vev --help show this help | |
| 77 | 77 | vev --version show version` | |
| 78 | 78 | ||
| @@ -140,6 +140,12 @@ func parseArgs(args []string) (command, error) { | |||
| 140 | 140 | if len(args) < 2 || args[1] == "" { | |
| 141 | 141 | return command{}, usagef("`kill` requires a session name, --all, or --daemon") | |
| 142 | 142 | } | |
| 143 | + | if args[1] == "--" { | |
| 144 | + | if len(args) != 3 || args[2] == "" { | |
| 145 | + | return command{}, usagef("`kill --` requires a session name") | |
| 146 | + | } | |
| 147 | + | return command{kind: kindKill, name: args[2]}, nil | |
| 148 | + | } | |
| 143 | 149 | if len(args) > 2 { | |
| 144 | 150 | return command{}, usagef("`kill` accepts exactly one session name, --all, or --daemon") | |
| 145 | 151 | } | |
| @@ -374,7 +380,7 @@ func printKillSuccess(name string, all, daemon bool) { | |||
| 374 | 380 | return | |
| 375 | 381 | } | |
| 376 | 382 | if all { | |
| 377 | - | fmt.Println("killed all sessions") | |
| 383 | + | fmt.Println("killed all sessions and stopped daemon") | |
| 378 | 384 | return | |
| 379 | 385 | } | |
| 380 | 386 | fmt.Printf("killed %s\n", name) | |
| @@ -71,8 +71,8 @@ usage: | |||||
| 71 | attach through SSH to a remote vev daemon | 71 | attach through SSH to a remote vev daemon | ||
| 72 | vev ls list sessions | 72 | vev ls list sessions | ||
| 73 | vev kill <name> kill a named session | 73 | vev kill <name> kill a named session | ||
| 74 | - | vev kill --all kill all sessions | 74 | + | vev kill --all kill all sessions and stop the daemon |
| 75 | - | vev kill --daemon kill the active vev daemon | 75 | + | vev kill --daemon stop the active vev daemon |
| 76 | vev --help show this help | 76 | vev --help show this help | ||
| 77 | vev --version show version` | 77 | vev --version show version` | ||
| 78 | 78 | ||||
| @@ -140,6 +140,12 @@ func parseArgs(args []string) (command, error) { | |||||
| 140 | if len(args) < 2 || args[1] == "" { | 140 | if len(args) < 2 || args[1] == "" { | ||
| 141 | return command{}, usagef("`kill` requires a session name, --all, or --daemon") | 141 | return command{}, usagef("`kill` requires a session name, --all, or --daemon") | ||
| 142 | } | 142 | } | ||
| 143 | + | if args[1] == "--" { | |||
| 144 | + | if len(args) != 3 || args[2] == "" { | |||
| 145 | + | return command{}, usagef("`kill --` requires a session name") | |||
| 146 | + | } | |||
| 147 | + | return command{kind: kindKill, name: args[2]}, nil | |||
| 148 | + | } | |||
| 143 | if len(args) > 2 { | 149 | if len(args) > 2 { | ||
| 144 | return command{}, usagef("`kill` accepts exactly one session name, --all, or --daemon") | 150 | return command{}, usagef("`kill` accepts exactly one session name, --all, or --daemon") | ||
| 145 | } | 151 | } | ||
| @@ -374,7 +380,7 @@ func printKillSuccess(name string, all, daemon bool) { | |||||
| 374 | return | 380 | return | ||
| 375 | } | 381 | } | ||
| 376 | if all { | 382 | if all { | ||
| 377 | - | fmt.Println("killed all sessions") | 383 | + | fmt.Println("killed all sessions and stopped daemon") |
| 378 | return | 384 | return | ||
| 379 | } | 385 | } | ||
| 380 | fmt.Printf("killed %s\n", name) | 386 | fmt.Printf("killed %s\n", name) | ||
M internal/app/run_test.go +2 -0
| @@ -35,9 +35,11 @@ func TestParseArgs(t *testing.T) { | |||
| 35 | 35 | {name: "ls", args: []string{"ls"}, wantKind: kindList}, | |
| 36 | 36 | {name: "list", args: []string{"list"}, wantKind: kindList}, | |
| 37 | 37 | {name: "kill named", args: []string{"kill", "work"}, wantKind: kindKill, wantName: "work"}, | |
| 38 | + | {name: "kill dashed name via terminator", args: []string{"kill", "--", "--all"}, wantKind: kindKill, wantName: "--all"}, | |
| 38 | 39 | {name: "kill all", args: []string{"kill", "--all"}, wantKind: kindKill, wantAll: true}, | |
| 39 | 40 | {name: "kill daemon", args: []string{"kill", "--daemon"}, wantKind: kindKill, wantDaemon: true}, | |
| 40 | 41 | {name: "kill without name", args: []string{"kill"}, wantErr: true}, | |
| 42 | + | {name: "kill terminator without name", args: []string{"kill", "--"}, wantErr: true}, | |
| 41 | 43 | {name: "kill all rejects extra arg", args: []string{"kill", "--all", "extra"}, wantErr: true}, | |
| 42 | 44 | {name: "kill daemon rejects extra arg", args: []string{"kill", "--daemon", "extra"}, wantErr: true}, | |
| 43 | 45 | {name: "kill extra arg", args: []string{"kill", "work", "extra"}, wantErr: true}, | |
| @@ -35,9 +35,11 @@ func TestParseArgs(t *testing.T) { | |||||
| 35 | {name: "ls", args: []string{"ls"}, wantKind: kindList}, | 35 | {name: "ls", args: []string{"ls"}, wantKind: kindList}, | ||
| 36 | {name: "list", args: []string{"list"}, wantKind: kindList}, | 36 | {name: "list", args: []string{"list"}, wantKind: kindList}, | ||
| 37 | {name: "kill named", args: []string{"kill", "work"}, wantKind: kindKill, wantName: "work"}, | 37 | {name: "kill named", args: []string{"kill", "work"}, wantKind: kindKill, wantName: "work"}, | ||
| 38 | + | {name: "kill dashed name via terminator", args: []string{"kill", "--", "--all"}, wantKind: kindKill, wantName: "--all"}, | |||
| 38 | {name: "kill all", args: []string{"kill", "--all"}, wantKind: kindKill, wantAll: true}, | 39 | {name: "kill all", args: []string{"kill", "--all"}, wantKind: kindKill, wantAll: true}, | ||
| 39 | {name: "kill daemon", args: []string{"kill", "--daemon"}, wantKind: kindKill, wantDaemon: true}, | 40 | {name: "kill daemon", args: []string{"kill", "--daemon"}, wantKind: kindKill, wantDaemon: true}, | ||
| 40 | {name: "kill without name", args: []string{"kill"}, wantErr: true}, | 41 | {name: "kill without name", args: []string{"kill"}, wantErr: true}, | ||
| 42 | + | {name: "kill terminator without name", args: []string{"kill", "--"}, wantErr: true}, | |||
| 41 | {name: "kill all rejects extra arg", args: []string{"kill", "--all", "extra"}, wantErr: true}, | 43 | {name: "kill all rejects extra arg", args: []string{"kill", "--all", "extra"}, wantErr: true}, | ||
| 42 | {name: "kill daemon rejects extra arg", args: []string{"kill", "--daemon", "extra"}, wantErr: true}, | 44 | {name: "kill daemon rejects extra arg", args: []string{"kill", "--daemon", "extra"}, wantErr: true}, | ||
| 43 | {name: "kill extra arg", args: []string{"kill", "work", "extra"}, wantErr: true}, | 45 | {name: "kill extra arg", args: []string{"kill", "work", "extra"}, wantErr: true}, | ||
M internal/ports/wire.go +0 -2
| @@ -430,8 +430,6 @@ func MarshalKill(m Kill) []byte { | |||
| 430 | 430 | w.putString(m.Name) | |
| 431 | 431 | if m.All { | |
| 432 | 432 | w.putUint8(1) | |
| 433 | - | } else { | |
| 434 | - | w.putUint8(0) | |
| 435 | 433 | } | |
| 436 | 434 | return w.b | |
| 437 | 435 | } | |
| @@ -430,8 +430,6 @@ func MarshalKill(m Kill) []byte { | |||||
| 430 | w.putString(m.Name) | 430 | w.putString(m.Name) | ||
| 431 | if m.All { | 431 | if m.All { | ||
| 432 | w.putUint8(1) | 432 | w.putUint8(1) | ||
| 433 | - | } else { | |||
| 434 | - | w.putUint8(0) | |||
| 435 | } | 433 | } | ||
| 436 | return w.b | 434 | return w.b | ||
| 437 | } | 435 | } | ||
M internal/ports/wire_test.go +4 -5
| @@ -319,8 +319,8 @@ func TestKillGoldenAndRoundTrip(t *testing.T) { | |||
| 319 | 319 | msg Kill | |
| 320 | 320 | want []byte | |
| 321 | 321 | }{ | |
| 322 | - | {name: "named", msg: Kill{Name: "main"}, want: []byte{0x00, 0x04, 0x6d, 0x61, 0x69, 0x6e, 0x00}}, | |
| 323 | - | {name: "empty", msg: Kill{Name: ""}, want: []byte{0x00, 0x00, 0x00}}, | |
| 322 | + | {name: "named", msg: Kill{Name: "main"}, want: []byte{0x00, 0x04, 0x6d, 0x61, 0x69, 0x6e}}, | |
| 323 | + | {name: "empty", msg: Kill{Name: ""}, want: []byte{0x00, 0x00}}, | |
| 324 | 324 | {name: "all", msg: Kill{All: true}, want: []byte{0x00, 0x00, 0x01}}, | |
| 325 | 325 | } | |
| 326 | 326 | ||
| @@ -340,8 +340,7 @@ func TestKillGoldenAndRoundTrip(t *testing.T) { | |||
| 340 | 340 | }) | |
| 341 | 341 | } | |
| 342 | 342 | ||
| 343 | - | full := MarshalKill(tests[0].msg) | |
| 344 | - | legacy := full[:len(full)-1] | |
| 343 | + | legacy := MarshalKill(tests[0].msg) | |
| 345 | 344 | back, err := UnmarshalKill(legacy) | |
| 346 | 345 | if err != nil { | |
| 347 | 346 | t.Fatalf("UnmarshalKill(legacy) error = %v", err) | |
| @@ -350,7 +349,7 @@ func TestKillGoldenAndRoundTrip(t *testing.T) { | |||
| 350 | 349 | t.Fatalf("legacy round trip = %#v, want named kill", back) | |
| 351 | 350 | } | |
| 352 | 351 | assertAllPrefixesFail(t, legacy, UnmarshalKill) | |
| 353 | - | assertTrailingGarbageFails(t, full, UnmarshalKill) | |
| 352 | + | assertTrailingGarbageFails(t, append(append([]byte(nil), legacy...), 0x00), UnmarshalKill) | |
| 354 | 353 | } | |
| 355 | 354 | ||
| 356 | 355 | func TestSessionsGoldenAndRoundTrip(t *testing.T) { | |
| @@ -319,8 +319,8 @@ func TestKillGoldenAndRoundTrip(t *testing.T) { | |||||
| 319 | msg Kill | 319 | msg Kill | ||
| 320 | want []byte | 320 | want []byte | ||
| 321 | }{ | 321 | }{ | ||
| 322 | - | {name: "named", msg: Kill{Name: "main"}, want: []byte{0x00, 0x04, 0x6d, 0x61, 0x69, 0x6e, 0x00}}, | 322 | + | {name: "named", msg: Kill{Name: "main"}, want: []byte{0x00, 0x04, 0x6d, 0x61, 0x69, 0x6e}}, |
| 323 | - | {name: "empty", msg: Kill{Name: ""}, want: []byte{0x00, 0x00, 0x00}}, | 323 | + | {name: "empty", msg: Kill{Name: ""}, want: []byte{0x00, 0x00}}, |
| 324 | {name: "all", msg: Kill{All: true}, want: []byte{0x00, 0x00, 0x01}}, | 324 | {name: "all", msg: Kill{All: true}, want: []byte{0x00, 0x00, 0x01}}, | ||
| 325 | } | 325 | } | ||
| 326 | 326 | ||||
| @@ -340,8 +340,7 @@ func TestKillGoldenAndRoundTrip(t *testing.T) { | |||||
| 340 | }) | 340 | }) | ||
| 341 | } | 341 | } | ||
| 342 | 342 | ||||
| 343 | - | full := MarshalKill(tests[0].msg) | 343 | + | legacy := MarshalKill(tests[0].msg) |
| 344 | - | legacy := full[:len(full)-1] | |||
| 345 | back, err := UnmarshalKill(legacy) | 344 | back, err := UnmarshalKill(legacy) | ||
| 346 | if err != nil { | 345 | if err != nil { | ||
| 347 | t.Fatalf("UnmarshalKill(legacy) error = %v", err) | 346 | t.Fatalf("UnmarshalKill(legacy) error = %v", err) | ||
| @@ -350,7 +349,7 @@ func TestKillGoldenAndRoundTrip(t *testing.T) { | |||||
| 350 | t.Fatalf("legacy round trip = %#v, want named kill", back) | 349 | t.Fatalf("legacy round trip = %#v, want named kill", back) | ||
| 351 | } | 350 | } | ||
| 352 | assertAllPrefixesFail(t, legacy, UnmarshalKill) | 351 | assertAllPrefixesFail(t, legacy, UnmarshalKill) | ||
| 353 | - | assertTrailingGarbageFails(t, full, UnmarshalKill) | 352 | + | assertTrailingGarbageFails(t, append(append([]byte(nil), legacy...), 0x00), UnmarshalKill) |
| 354 | } | 353 | } | ||
| 355 | 354 | ||||
| 356 | func TestSessionsGoldenAndRoundTrip(t *testing.T) { | 355 | func TestSessionsGoldenAndRoundTrip(t *testing.T) { | ||