fix(remote): address review findings
- commit
- 2f5b15ccb375172ad0defee0cf1f12cabbefdce5
- author
- brice · · 2026-07-06 21:40 UTC
- parent
- d737ef0
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +17 -9
M internal/adapters/dgram/ssh_dialer.go +4 -0
| @@ -113,6 +113,10 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||
| 113 | 113 | ||
| 114 | 114 | ready, err := readUDPReady(stdout) | |
| 115 | 115 | if err != nil { | |
| 116 | + | _ = proc.Kill() | |
| 117 | + | _ = proc.Wait() | |
| 118 | + | waited = true | |
| 119 | + | cleanup = false | |
| 116 | 120 | return nil, udpUnavailable("read bootstrap readiness", err, &stderr) | |
| 117 | 121 | } | |
| 118 | 122 | if err := proc.Wait(); err != nil { | |
| @@ -113,6 +113,10 @@ func (d RemoteDialer) Dial(ctx context.Context) (ports.Transport, error) { | |||||
| 113 | 113 | ||||
| 114 | ready, err := readUDPReady(stdout) | 114 | ready, err := readUDPReady(stdout) | ||
| 115 | if err != nil { | 115 | if err != nil { | ||
| 116 | + | _ = proc.Kill() | |||
| 117 | + | _ = proc.Wait() | |||
| 118 | + | waited = true | |||
| 119 | + | cleanup = false | |||
| 116 | return nil, udpUnavailable("read bootstrap readiness", err, &stderr) | 120 | return nil, udpUnavailable("read bootstrap readiness", err, &stderr) | ||
| 117 | } | 121 | } | ||
| 118 | if err := proc.Wait(); err != nil { | 122 | if err := proc.Wait(); err != nil { | ||
M internal/adapters/remote/remote_factory.go +2 -0
| @@ -13,6 +13,8 @@ import ( | |||
| 13 | 13 | // DialerFactory selects the requested remote transport adapter. | |
| 14 | 14 | type DialerFactory struct{} | |
| 15 | 15 | ||
| 16 | + | var _ ports.RemoteDialerFactory = DialerFactory{} | |
| 17 | + | ||
| 16 | 18 | func NewDialerFactory() DialerFactory { return DialerFactory{} } | |
| 17 | 19 | ||
| 18 | 20 | func (DialerFactory) DialerForRemote(target string, session string, mode ports.RemoteTransportMode, log *slog.Logger) (ports.Dialer, error) { | |
| @@ -13,6 +13,8 @@ import ( | |||||
| 13 | // DialerFactory selects the requested remote transport adapter. | 13 | // DialerFactory selects the requested remote transport adapter. | ||
| 14 | type DialerFactory struct{} | 14 | type DialerFactory struct{} | ||
| 15 | 15 | ||||
| 16 | + | var _ ports.RemoteDialerFactory = DialerFactory{} | |||
| 17 | + | ||||
| 16 | func NewDialerFactory() DialerFactory { return DialerFactory{} } | 18 | func NewDialerFactory() DialerFactory { return DialerFactory{} } | ||
| 17 | 19 | ||||
| 18 | func (DialerFactory) DialerForRemote(target string, session string, mode ports.RemoteTransportMode, log *slog.Logger) (ports.Dialer, error) { | 20 | func (DialerFactory) DialerForRemote(target string, session string, mode ports.RemoteTransportMode, log *slog.Logger) (ports.Dialer, error) { | ||
M internal/app/remote_test.go +1 -0
| @@ -20,6 +20,7 @@ func TestParseRemoteAttachTarget(t *testing.T) { | |||
| 20 | 20 | {name: "bracketed ipv6 no session", input: "user@[2001:db8::1]", wantTarget: "user@[2001:db8::1]", wantOK: true}, | |
| 21 | 21 | {name: "bracketed ipv6 with session", input: "user@[2001:db8::1]:work", wantTarget: "user@[2001:db8::1]", wantSession: "work", wantOK: true}, | |
| 22 | 22 | {name: "malformed bracketed ipv6", input: "user@[2001:db8::1"}, | |
| 23 | + | {name: "malformed suffix after bracket", input: "user@[2001:db8::1]work"}, | |
| 23 | 24 | {name: "target with at in userinfo", input: "first@user@example.com:work", wantTarget: "first@user@example.com", wantSession: "work", wantOK: true}, | |
| 24 | 25 | } | |
| 25 | 26 | ||
| @@ -20,6 +20,7 @@ func TestParseRemoteAttachTarget(t *testing.T) { | |||||
| 20 | {name: "bracketed ipv6 no session", input: "user@[2001:db8::1]", wantTarget: "user@[2001:db8::1]", wantOK: true}, | 20 | {name: "bracketed ipv6 no session", input: "user@[2001:db8::1]", wantTarget: "user@[2001:db8::1]", wantOK: true}, | ||
| 21 | {name: "bracketed ipv6 with session", input: "user@[2001:db8::1]:work", wantTarget: "user@[2001:db8::1]", wantSession: "work", wantOK: true}, | 21 | {name: "bracketed ipv6 with session", input: "user@[2001:db8::1]:work", wantTarget: "user@[2001:db8::1]", wantSession: "work", wantOK: true}, | ||
| 22 | {name: "malformed bracketed ipv6", input: "user@[2001:db8::1"}, | 22 | {name: "malformed bracketed ipv6", input: "user@[2001:db8::1"}, | ||
| 23 | + | {name: "malformed suffix after bracket", input: "user@[2001:db8::1]work"}, | |||
| 23 | {name: "target with at in userinfo", input: "first@user@example.com:work", wantTarget: "first@user@example.com", wantSession: "work", wantOK: true}, | 24 | {name: "target with at in userinfo", input: "first@user@example.com:work", wantTarget: "first@user@example.com", wantSession: "work", wantOK: true}, | ||
| 24 | } | 25 | } | ||
| 25 | 26 | ||||
M internal/app/run_test.go +10 -9
| @@ -466,7 +466,11 @@ func TestRunAttachWithDepsSelectsRemoteTransport(t *testing.T) { | |||
| 466 | 466 | selectedRemoteTransport: tt.selectedTransport, | |
| 467 | 467 | clipboard: clip, | |
| 468 | 468 | runClient: func(_ context.Context, d ports.Dialer, _ ports.Terminal, _ ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, _ *slog.Logger) error { | |
| 469 | - | gotDialer = d.(namedDialer).name | |
| 469 | + | nd, ok := d.(namedDialer) | |
| 470 | + | if !ok { | |
| 471 | + | t.Fatalf("dialer type = %T, want namedDialer", d) | |
| 472 | + | } | |
| 473 | + | gotDialer = nd.name | |
| 470 | 474 | gotRemote = remote | |
| 471 | 475 | gotClipboard = clipboard | |
| 472 | 476 | if intent != ports.IntentAttach || name != "work" { | |
| @@ -542,7 +546,11 @@ func TestRunAttachWithDepsBuildsLocalDialer(t *testing.T) { | |||
| 542 | 546 | remoteDialerFactory: factory, | |
| 543 | 547 | clipboard: &fakeClipboardReader{}, // must NOT reach runClient for a local attach | |
| 544 | 548 | runClient: func(_ context.Context, d ports.Dialer, _ ports.Terminal, _ ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, _ *slog.Logger) error { | |
| 545 | - | gotDialer = d.(namedDialer).name | |
| 549 | + | nd, ok := d.(namedDialer) | |
| 550 | + | if !ok { | |
| 551 | + | t.Fatalf("dialer type = %T, want namedDialer", d) | |
| 552 | + | } | |
| 553 | + | gotDialer = nd.name | |
| 546 | 554 | gotRemote = remote | |
| 547 | 555 | gotClipboard = clipboard | |
| 548 | 556 | if intent != ports.IntentEphemeral || name != "" { | |
| @@ -570,20 +578,13 @@ func TestRunUDPBootstrapForwardsReadinessAndExits(t *testing.T) { | |||
| 570 | 578 | udpBootstrapTimeout = time.Second | |
| 571 | 579 | t.Cleanup(func() { udpBootstrapTimeout = oldTimeout }) | |
| 572 | 580 | oldCommand := udpProxyCommand | |
| 573 | - | r, w, err := os.Pipe() | |
| 574 | - | if err != nil { | |
| 575 | - | t.Fatal(err) | |
| 576 | - | } | |
| 577 | - | defer func() { _ = r.Close() }() | |
| 578 | 581 | var gotCmd *exec.Cmd | |
| 579 | 582 | udpProxyCommand = func(context.Context, string, ...string) *exec.Cmd { | |
| 580 | 583 | cmd := exec.Command("/bin/sh", "-c", "printf 'VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\\n'") | |
| 581 | - | cmd.Stdout = w | |
| 582 | 584 | gotCmd = cmd | |
| 583 | 585 | return cmd | |
| 584 | 586 | } | |
| 585 | 587 | t.Cleanup(func() { udpProxyCommand = oldCommand }) | |
| 586 | - | defer func() { _ = w.Close() }() | |
| 587 | 588 | ||
| 588 | 589 | got := captureStdout(t, func() { | |
| 589 | 590 | if err := runUDPBootstrap(context.Background(), "work"); err != nil { | |
| @@ -466,7 +466,11 @@ func TestRunAttachWithDepsSelectsRemoteTransport(t *testing.T) { | |||||
| 466 | selectedRemoteTransport: tt.selectedTransport, | 466 | selectedRemoteTransport: tt.selectedTransport, | ||
| 467 | clipboard: clip, | 467 | clipboard: clip, | ||
| 468 | runClient: func(_ context.Context, d ports.Dialer, _ ports.Terminal, _ ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, _ *slog.Logger) error { | 468 | runClient: func(_ context.Context, d ports.Dialer, _ ports.Terminal, _ ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, _ *slog.Logger) error { | ||
| 469 | - | gotDialer = d.(namedDialer).name | 469 | + | nd, ok := d.(namedDialer) |
| 470 | + | if !ok { | |||
| 471 | + | t.Fatalf("dialer type = %T, want namedDialer", d) | |||
| 472 | + | } | |||
| 473 | + | gotDialer = nd.name | |||
| 470 | gotRemote = remote | 474 | gotRemote = remote | ||
| 471 | gotClipboard = clipboard | 475 | gotClipboard = clipboard | ||
| 472 | if intent != ports.IntentAttach || name != "work" { | 476 | if intent != ports.IntentAttach || name != "work" { | ||
| @@ -542,7 +546,11 @@ func TestRunAttachWithDepsBuildsLocalDialer(t *testing.T) { | |||||
| 542 | remoteDialerFactory: factory, | 546 | remoteDialerFactory: factory, | ||
| 543 | clipboard: &fakeClipboardReader{}, // must NOT reach runClient for a local attach | 547 | clipboard: &fakeClipboardReader{}, // must NOT reach runClient for a local attach | ||
| 544 | runClient: func(_ context.Context, d ports.Dialer, _ ports.Terminal, _ ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, _ *slog.Logger) error { | 548 | runClient: func(_ context.Context, d ports.Dialer, _ ports.Terminal, _ ports.Clock, intent uint8, name string, remote bool, clipboard ports.ClipboardReader, _ *slog.Logger) error { | ||
| 545 | - | gotDialer = d.(namedDialer).name | 549 | + | nd, ok := d.(namedDialer) |
| 550 | + | if !ok { | |||
| 551 | + | t.Fatalf("dialer type = %T, want namedDialer", d) | |||
| 552 | + | } | |||
| 553 | + | gotDialer = nd.name | |||
| 546 | gotRemote = remote | 554 | gotRemote = remote | ||
| 547 | gotClipboard = clipboard | 555 | gotClipboard = clipboard | ||
| 548 | if intent != ports.IntentEphemeral || name != "" { | 556 | if intent != ports.IntentEphemeral || name != "" { | ||
| @@ -570,20 +578,13 @@ func TestRunUDPBootstrapForwardsReadinessAndExits(t *testing.T) { | |||||
| 570 | udpBootstrapTimeout = time.Second | 578 | udpBootstrapTimeout = time.Second | ||
| 571 | t.Cleanup(func() { udpBootstrapTimeout = oldTimeout }) | 579 | t.Cleanup(func() { udpBootstrapTimeout = oldTimeout }) | ||
| 572 | oldCommand := udpProxyCommand | 580 | oldCommand := udpProxyCommand | ||
| 573 | - | r, w, err := os.Pipe() | |||
| 574 | - | if err != nil { | |||
| 575 | - | t.Fatal(err) | |||
| 576 | - | } | |||
| 577 | - | defer func() { _ = r.Close() }() | |||
| 578 | var gotCmd *exec.Cmd | 581 | var gotCmd *exec.Cmd | ||
| 579 | udpProxyCommand = func(context.Context, string, ...string) *exec.Cmd { | 582 | udpProxyCommand = func(context.Context, string, ...string) *exec.Cmd { | ||
| 580 | cmd := exec.Command("/bin/sh", "-c", "printf 'VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\\n'") | 583 | cmd := exec.Command("/bin/sh", "-c", "printf 'VEV-UDP 4242 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=\\n'") | ||
| 581 | - | cmd.Stdout = w | |||
| 582 | gotCmd = cmd | 584 | gotCmd = cmd | ||
| 583 | return cmd | 585 | return cmd | ||
| 584 | } | 586 | } | ||
| 585 | t.Cleanup(func() { udpProxyCommand = oldCommand }) | 587 | t.Cleanup(func() { udpProxyCommand = oldCommand }) | ||
| 586 | - | defer func() { _ = w.Close() }() | |||
| 587 | 588 | ||||
| 588 | got := captureStdout(t, func() { | 589 | got := captureStdout(t, func() { | ||
| 589 | if err := runUDPBootstrap(context.Background(), "work"); err != nil { | 590 | if err := runUDPBootstrap(context.Background(), "work"); err != nil { | ||