fix(app): bound stale daemon recovery
- commit
- 7281253e8cafbd52473cfd4bc8c0c6d2b3f748f6
- author
- brice · · 2026-07-03 12:14 UTC
- parent
- 812d54f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +191 -44
M internal/app/run.go +68 -18
| @@ -16,6 +16,7 @@ import ( | |||
| 16 | 16 | "os/signal" | |
| 17 | 17 | "syscall" | |
| 18 | 18 | "text/tabwriter" | |
| 19 | + | "time" | |
| 19 | 20 | ||
| 20 | 21 | "github.com/bnema/vev/internal/adapters/clock" | |
| 21 | 22 | "github.com/bnema/vev/internal/adapters/ipc" | |
| @@ -254,18 +255,26 @@ func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) err | |||
| 254 | 255 | } | |
| 255 | 256 | ||
| 256 | 257 | return runLocalAttachWithRecovery(ctx, intent, name, attachRecoveryDeps{ | |
| 257 | - | confirmer: confirm.NewConfirmer(os.Stdin, os.Stderr), | |
| 258 | - | attach: attachLocalDaemon, | |
| 259 | - | killDaemon: requestDaemonStop, | |
| 258 | + | confirmer: confirm.NewConfirmer(os.Stdin, os.Stderr), | |
| 259 | + | attach: attachLocalDaemon, | |
| 260 | + | killDaemon: requestDaemonStop, | |
| 261 | + | settleAfterKill: waitForDaemonStop, | |
| 260 | 262 | }) | |
| 261 | 263 | } | |
| 262 | 264 | ||
| 263 | 265 | type attachRecoveryDeps struct { | |
| 264 | - | confirmer confirm.Confirmer | |
| 265 | - | attach func(context.Context, uint8, string) error | |
| 266 | - | killDaemon func(context.Context) error | |
| 266 | + | confirmer confirm.Confirmer | |
| 267 | + | attach func(context.Context, uint8, string) error | |
| 268 | + | killDaemon func(context.Context) error | |
| 269 | + | settleAfterKill func(context.Context) error | |
| 267 | 270 | } | |
| 268 | 271 | ||
| 272 | + | const ( | |
| 273 | + | daemonStopTimeout = 2 * time.Second | |
| 274 | + | daemonRestartSettle = 50 * time.Millisecond | |
| 275 | + | legacyMalformedHelloSignal = "malformed hello" | |
| 276 | + | ) | |
| 277 | + | ||
| 269 | 278 | func runLocalAttachWithRecovery(ctx context.Context, intent uint8, name string, deps attachRecoveryDeps) error { | |
| 270 | 279 | err := deps.attach(ctx, intent, name) | |
| 271 | 280 | if !isDaemonVersionDrift(err) { | |
| @@ -282,6 +291,11 @@ func runLocalAttachWithRecovery(ctx context.Context, intent uint8, name string, | |||
| 282 | 291 | if killErr := deps.killDaemon(ctx); killErr != nil { | |
| 283 | 292 | return killErr | |
| 284 | 293 | } | |
| 294 | + | if deps.settleAfterKill != nil { | |
| 295 | + | if settleErr := deps.settleAfterKill(ctx); settleErr != nil { | |
| 296 | + | return settleErr | |
| 297 | + | } | |
| 298 | + | } | |
| 285 | 299 | return deps.attach(ctx, intent, name) | |
| 286 | 300 | } | |
| 287 | 301 | ||
| @@ -290,7 +304,7 @@ func isDaemonVersionDrift(err error) bool { | |||
| 290 | 304 | if !errors.As(err, &protocolErr) { | |
| 291 | 305 | return false | |
| 292 | 306 | } | |
| 293 | - | return protocolErr.Code == ports.ErrVersionMismatch || protocolErr.Text == "malformed hello" | |
| 307 | + | return protocolErr.Code == ports.ErrVersionMismatch || protocolErr.Text == legacyMalformedHelloSignal | |
| 294 | 308 | } | |
| 295 | 309 | ||
| 296 | 310 | func attachLocalDaemon(ctx context.Context, intent uint8, name string) error { | |
| @@ -302,19 +316,55 @@ func attachLocalDaemon(ctx context.Context, intent uint8, name string) error { | |||
| 302 | 316 | return client.Attach(ctx, transport, term.New(), intent, name) | |
| 303 | 317 | } | |
| 304 | 318 | ||
| 305 | - | func requestDaemonStop(_ context.Context) error { | |
| 306 | - | transport, err := realDial(ipc.SocketDir()) | |
| 307 | - | if err != nil { | |
| 308 | - | return fmt.Errorf("vev: no daemon running") | |
| 309 | - | } | |
| 310 | - | defer func() { _ = transport.Close() }() | |
| 311 | - | if err := transport.Send(ports.Frame{Type: ports.MsgKill, Payload: ports.MarshalKill(ports.Kill{All: true})}); err != nil { | |
| 312 | - | return fmt.Errorf("vev: requesting daemon stop: %w", err) | |
| 319 | + | func waitForDaemonStop(ctx context.Context) error { | |
| 320 | + | timer := time.NewTimer(daemonRestartSettle) | |
| 321 | + | defer timer.Stop() | |
| 322 | + | select { | |
| 323 | + | case <-ctx.Done(): | |
| 324 | + | return ctx.Err() | |
| 325 | + | case <-timer.C: | |
| 326 | + | return nil | |
| 313 | 327 | } | |
| 314 | - | if _, err := transport.Recv(); err != nil && !errors.Is(err, io.EOF) { | |
| 315 | - | return fmt.Errorf("vev: reading daemon stop reply: %w", err) | |
| 328 | + | } | |
| 329 | + | ||
| 330 | + | func requestDaemonStop(ctx context.Context) error { | |
| 331 | + | ctx, cancel := context.WithTimeout(ctx, daemonStopTimeout) | |
| 332 | + | defer cancel() | |
| 333 | + | ||
| 334 | + | done := make(chan error, 1) | |
| 335 | + | go func() { | |
| 336 | + | transport, err := realDial(ipc.SocketDir()) | |
| 337 | + | if err != nil { | |
| 338 | + | done <- fmt.Errorf("vev: no daemon running") | |
| 339 | + | return | |
| 340 | + | } | |
| 341 | + | defer func() { _ = transport.Close() }() | |
| 342 | + | closed := make(chan struct{}) | |
| 343 | + | go func() { | |
| 344 | + | select { | |
| 345 | + | case <-ctx.Done(): | |
| 346 | + | _ = transport.Close() | |
| 347 | + | case <-closed: | |
| 348 | + | } | |
| 349 | + | }() | |
| 350 | + | defer close(closed) | |
| 351 | + | if err := transport.Send(ports.Frame{Type: ports.MsgKill, Payload: ports.MarshalKill(ports.Kill{All: true})}); err != nil { | |
| 352 | + | done <- fmt.Errorf("vev: requesting daemon stop: %w", err) | |
| 353 | + | return | |
| 354 | + | } | |
| 355 | + | if _, err := transport.Recv(); err != nil && !errors.Is(err, io.EOF) { | |
| 356 | + | done <- fmt.Errorf("vev: reading daemon stop reply: %w", err) | |
| 357 | + | return | |
| 358 | + | } | |
| 359 | + | done <- nil | |
| 360 | + | }() | |
| 361 | + | ||
| 362 | + | select { | |
| 363 | + | case err := <-done: | |
| 364 | + | return err | |
| 365 | + | case <-ctx.Done(): | |
| 366 | + | return fmt.Errorf("vev: stopping daemon: %w", ctx.Err()) | |
| 316 | 367 | } | |
| 317 | - | return nil | |
| 318 | 368 | } | |
| 319 | 369 | ||
| 320 | 370 | // runStdio is the hidden remote-side mode used by `ssh host vev _stdio`: it | |
| @@ -16,6 +16,7 @@ import ( | |||||
| 16 | "os/signal" | 16 | "os/signal" | ||
| 17 | "syscall" | 17 | "syscall" | ||
| 18 | "text/tabwriter" | 18 | "text/tabwriter" | ||
| 19 | + | "time" | |||
| 19 | 20 | ||||
| 20 | "github.com/bnema/vev/internal/adapters/clock" | 21 | "github.com/bnema/vev/internal/adapters/clock" | ||
| 21 | "github.com/bnema/vev/internal/adapters/ipc" | 22 | "github.com/bnema/vev/internal/adapters/ipc" | ||
| @@ -254,18 +255,26 @@ func runAttach(ctx context.Context, intent uint8, name, remoteTarget string) err | |||||
| 254 | } | 255 | } | ||
| 255 | 256 | ||||
| 256 | return runLocalAttachWithRecovery(ctx, intent, name, attachRecoveryDeps{ | 257 | return runLocalAttachWithRecovery(ctx, intent, name, attachRecoveryDeps{ | ||
| 257 | - | confirmer: confirm.NewConfirmer(os.Stdin, os.Stderr), | 258 | + | confirmer: confirm.NewConfirmer(os.Stdin, os.Stderr), |
| 258 | - | attach: attachLocalDaemon, | 259 | + | attach: attachLocalDaemon, |
| 259 | - | killDaemon: requestDaemonStop, | 260 | + | killDaemon: requestDaemonStop, |
| 261 | + | settleAfterKill: waitForDaemonStop, | |||
| 260 | }) | 262 | }) | ||
| 261 | } | 263 | } | ||
| 262 | 264 | ||||
| 263 | type attachRecoveryDeps struct { | 265 | type attachRecoveryDeps struct { | ||
| 264 | - | confirmer confirm.Confirmer | 266 | + | confirmer confirm.Confirmer |
| 265 | - | attach func(context.Context, uint8, string) error | 267 | + | attach func(context.Context, uint8, string) error |
| 266 | - | killDaemon func(context.Context) error | 268 | + | killDaemon func(context.Context) error |
| 269 | + | settleAfterKill func(context.Context) error | |||
| 267 | } | 270 | } | ||
| 268 | 271 | ||||
| 272 | + | const ( | |||
| 273 | + | daemonStopTimeout = 2 * time.Second | |||
| 274 | + | daemonRestartSettle = 50 * time.Millisecond | |||
| 275 | + | legacyMalformedHelloSignal = "malformed hello" | |||
| 276 | + | ) | |||
| 277 | + | ||||
| 269 | func runLocalAttachWithRecovery(ctx context.Context, intent uint8, name string, deps attachRecoveryDeps) error { | 278 | func runLocalAttachWithRecovery(ctx context.Context, intent uint8, name string, deps attachRecoveryDeps) error { | ||
| 270 | err := deps.attach(ctx, intent, name) | 279 | err := deps.attach(ctx, intent, name) | ||
| 271 | if !isDaemonVersionDrift(err) { | 280 | if !isDaemonVersionDrift(err) { | ||
| @@ -282,6 +291,11 @@ func runLocalAttachWithRecovery(ctx context.Context, intent uint8, name string, | |||||
| 282 | if killErr := deps.killDaemon(ctx); killErr != nil { | 291 | if killErr := deps.killDaemon(ctx); killErr != nil { | ||
| 283 | return killErr | 292 | return killErr | ||
| 284 | } | 293 | } | ||
| 294 | + | if deps.settleAfterKill != nil { | |||
| 295 | + | if settleErr := deps.settleAfterKill(ctx); settleErr != nil { | |||
| 296 | + | return settleErr | |||
| 297 | + | } | |||
| 298 | + | } | |||
| 285 | return deps.attach(ctx, intent, name) | 299 | return deps.attach(ctx, intent, name) | ||
| 286 | } | 300 | } | ||
| 287 | 301 | ||||
| @@ -290,7 +304,7 @@ func isDaemonVersionDrift(err error) bool { | |||||
| 290 | if !errors.As(err, &protocolErr) { | 304 | if !errors.As(err, &protocolErr) { | ||
| 291 | return false | 305 | return false | ||
| 292 | } | 306 | } | ||
| 293 | - | return protocolErr.Code == ports.ErrVersionMismatch || protocolErr.Text == "malformed hello" | 307 | + | return protocolErr.Code == ports.ErrVersionMismatch || protocolErr.Text == legacyMalformedHelloSignal |
| 294 | } | 308 | } | ||
| 295 | 309 | ||||
| 296 | func attachLocalDaemon(ctx context.Context, intent uint8, name string) error { | 310 | func attachLocalDaemon(ctx context.Context, intent uint8, name string) error { | ||
| @@ -302,19 +316,55 @@ func attachLocalDaemon(ctx context.Context, intent uint8, name string) error { | |||||
| 302 | return client.Attach(ctx, transport, term.New(), intent, name) | 316 | return client.Attach(ctx, transport, term.New(), intent, name) | ||
| 303 | } | 317 | } | ||
| 304 | 318 | ||||
| 305 | - | func requestDaemonStop(_ context.Context) error { | 319 | + | func waitForDaemonStop(ctx context.Context) error { |
| 306 | - | transport, err := realDial(ipc.SocketDir()) | 320 | + | timer := time.NewTimer(daemonRestartSettle) |
| 307 | - | if err != nil { | 321 | + | defer timer.Stop() |
| 308 | - | return fmt.Errorf("vev: no daemon running") | 322 | + | select { |
| 309 | - | } | 323 | + | case <-ctx.Done(): |
| 310 | - | defer func() { _ = transport.Close() }() | 324 | + | return ctx.Err() |
| 311 | - | if err := transport.Send(ports.Frame{Type: ports.MsgKill, Payload: ports.MarshalKill(ports.Kill{All: true})}); err != nil { | 325 | + | case <-timer.C: |
| 312 | - | return fmt.Errorf("vev: requesting daemon stop: %w", err) | 326 | + | return nil |
| 313 | } | 327 | } | ||
| 314 | - | if _, err := transport.Recv(); err != nil && !errors.Is(err, io.EOF) { | 328 | + | } |
| 315 | - | return fmt.Errorf("vev: reading daemon stop reply: %w", err) | 329 | + | |
| 330 | + | func requestDaemonStop(ctx context.Context) error { | |||
| 331 | + | ctx, cancel := context.WithTimeout(ctx, daemonStopTimeout) | |||
| 332 | + | defer cancel() | |||
| 333 | + | ||||
| 334 | + | done := make(chan error, 1) | |||
| 335 | + | go func() { | |||
| 336 | + | transport, err := realDial(ipc.SocketDir()) | |||
| 337 | + | if err != nil { | |||
| 338 | + | done <- fmt.Errorf("vev: no daemon running") | |||
| 339 | + | return | |||
| 340 | + | } | |||
| 341 | + | defer func() { _ = transport.Close() }() | |||
| 342 | + | closed := make(chan struct{}) | |||
| 343 | + | go func() { | |||
| 344 | + | select { | |||
| 345 | + | case <-ctx.Done(): | |||
| 346 | + | _ = transport.Close() | |||
| 347 | + | case <-closed: | |||
| 348 | + | } | |||
| 349 | + | }() | |||
| 350 | + | defer close(closed) | |||
| 351 | + | if err := transport.Send(ports.Frame{Type: ports.MsgKill, Payload: ports.MarshalKill(ports.Kill{All: true})}); err != nil { | |||
| 352 | + | done <- fmt.Errorf("vev: requesting daemon stop: %w", err) | |||
| 353 | + | return | |||
| 354 | + | } | |||
| 355 | + | if _, err := transport.Recv(); err != nil && !errors.Is(err, io.EOF) { | |||
| 356 | + | done <- fmt.Errorf("vev: reading daemon stop reply: %w", err) | |||
| 357 | + | return | |||
| 358 | + | } | |||
| 359 | + | done <- nil | |||
| 360 | + | }() | |||
| 361 | + | ||||
| 362 | + | select { | |||
| 363 | + | case err := <-done: | |||
| 364 | + | return err | |||
| 365 | + | case <-ctx.Done(): | |||
| 366 | + | return fmt.Errorf("vev: stopping daemon: %w", ctx.Err()) | |||
| 316 | } | 367 | } | ||
| 317 | - | return nil | |||
| 318 | } | 368 | } | ||
| 319 | 369 | ||||
| 320 | // runStdio is the hidden remote-side mode used by `ssh host vev _stdio`: it | 370 | // runStdio is the hidden remote-side mode used by `ssh host vev _stdio`: it | ||
M internal/app/run_test.go +63 -0
| @@ -130,6 +130,69 @@ func TestRunLocalAttachPromptsAndRestartsOnProtocolMismatch(t *testing.T) { | |||
| 130 | 130 | } | |
| 131 | 131 | } | |
| 132 | 132 | ||
| 133 | + | func TestRunLocalAttachPropagatesPromptError(t *testing.T) { | |
| 134 | + | readErr := errors.New("read failed") | |
| 135 | + | err := runLocalAttachWithRecovery(context.Background(), ports.IntentEphemeral, "", attachRecoveryDeps{ | |
| 136 | + | confirmer: confirm.NewConfirmer(errorReader{err: readErr}, &bytes.Buffer{}), | |
| 137 | + | attach: func(context.Context, uint8, string) error { | |
| 138 | + | return &client.ProtocolError{Code: ports.ErrVersionMismatch, Text: "protocol version mismatch"} | |
| 139 | + | }, | |
| 140 | + | killDaemon: func(context.Context) error { | |
| 141 | + | t.Fatal("killDaemon should not be called after prompt error") | |
| 142 | + | return nil | |
| 143 | + | }, | |
| 144 | + | }) | |
| 145 | + | if !errors.Is(err, readErr) { | |
| 146 | + | t.Fatalf("error = %v, want wrapped %v", err, readErr) | |
| 147 | + | } | |
| 148 | + | } | |
| 149 | + | ||
| 150 | + | func TestRunLocalAttachPropagatesKillError(t *testing.T) { | |
| 151 | + | killErr := errors.New("kill failed") | |
| 152 | + | err := runLocalAttachWithRecovery(context.Background(), ports.IntentEphemeral, "", attachRecoveryDeps{ | |
| 153 | + | confirmer: confirm.NewConfirmer(strings.NewReader("y\n"), &bytes.Buffer{}), | |
| 154 | + | attach: func(context.Context, uint8, string) error { | |
| 155 | + | return &client.ProtocolError{Code: ports.ErrVersionMismatch, Text: "protocol version mismatch"} | |
| 156 | + | }, | |
| 157 | + | killDaemon: func(context.Context) error { return killErr }, | |
| 158 | + | }) | |
| 159 | + | if !errors.Is(err, killErr) { | |
| 160 | + | t.Fatalf("error = %v, want %v", err, killErr) | |
| 161 | + | } | |
| 162 | + | } | |
| 163 | + | ||
| 164 | + | func TestRunLocalAttachSettlesBeforeRetry(t *testing.T) { | |
| 165 | + | var order []string | |
| 166 | + | err := runLocalAttachWithRecovery(context.Background(), ports.IntentEphemeral, "", attachRecoveryDeps{ | |
| 167 | + | confirmer: confirm.NewConfirmer(strings.NewReader("y\n"), &bytes.Buffer{}), | |
| 168 | + | attach: func(context.Context, uint8, string) error { | |
| 169 | + | order = append(order, "attach") | |
| 170 | + | if len(order) == 1 { | |
| 171 | + | return &client.ProtocolError{Code: ports.ErrVersionMismatch, Text: "protocol version mismatch"} | |
| 172 | + | } | |
| 173 | + | return nil | |
| 174 | + | }, | |
| 175 | + | killDaemon: func(context.Context) error { | |
| 176 | + | order = append(order, "kill") | |
| 177 | + | return nil | |
| 178 | + | }, | |
| 179 | + | settleAfterKill: func(context.Context) error { | |
| 180 | + | order = append(order, "settle") | |
| 181 | + | return nil | |
| 182 | + | }, | |
| 183 | + | }) | |
| 184 | + | if err != nil { | |
| 185 | + | t.Fatalf("runLocalAttachWithRecovery returned error: %v", err) | |
| 186 | + | } | |
| 187 | + | if got, want := strings.Join(order, ","), "attach,kill,settle,attach"; got != want { | |
| 188 | + | t.Fatalf("order = %s, want %s", got, want) | |
| 189 | + | } | |
| 190 | + | } | |
| 191 | + | ||
| 192 | + | type errorReader struct{ err error } | |
| 193 | + | ||
| 194 | + | func (r errorReader) Read([]byte) (int, error) { return 0, r.err } | |
| 195 | + | ||
| 133 | 196 | func TestRunLocalAttachDeclineKeepsOriginalError(t *testing.T) { | |
| 134 | 197 | answers := strings.NewReader("n\n") | |
| 135 | 198 | wantErr := &client.ProtocolError{Code: ports.ErrInternal, Text: "malformed hello"} | |
| @@ -130,6 +130,69 @@ func TestRunLocalAttachPromptsAndRestartsOnProtocolMismatch(t *testing.T) { | |||||
| 130 | } | 130 | } | ||
| 131 | } | 131 | } | ||
| 132 | 132 | ||||
| 133 | + | func TestRunLocalAttachPropagatesPromptError(t *testing.T) { | |||
| 134 | + | readErr := errors.New("read failed") | |||
| 135 | + | err := runLocalAttachWithRecovery(context.Background(), ports.IntentEphemeral, "", attachRecoveryDeps{ | |||
| 136 | + | confirmer: confirm.NewConfirmer(errorReader{err: readErr}, &bytes.Buffer{}), | |||
| 137 | + | attach: func(context.Context, uint8, string) error { | |||
| 138 | + | return &client.ProtocolError{Code: ports.ErrVersionMismatch, Text: "protocol version mismatch"} | |||
| 139 | + | }, | |||
| 140 | + | killDaemon: func(context.Context) error { | |||
| 141 | + | t.Fatal("killDaemon should not be called after prompt error") | |||
| 142 | + | return nil | |||
| 143 | + | }, | |||
| 144 | + | }) | |||
| 145 | + | if !errors.Is(err, readErr) { | |||
| 146 | + | t.Fatalf("error = %v, want wrapped %v", err, readErr) | |||
| 147 | + | } | |||
| 148 | + | } | |||
| 149 | + | ||||
| 150 | + | func TestRunLocalAttachPropagatesKillError(t *testing.T) { | |||
| 151 | + | killErr := errors.New("kill failed") | |||
| 152 | + | err := runLocalAttachWithRecovery(context.Background(), ports.IntentEphemeral, "", attachRecoveryDeps{ | |||
| 153 | + | confirmer: confirm.NewConfirmer(strings.NewReader("y\n"), &bytes.Buffer{}), | |||
| 154 | + | attach: func(context.Context, uint8, string) error { | |||
| 155 | + | return &client.ProtocolError{Code: ports.ErrVersionMismatch, Text: "protocol version mismatch"} | |||
| 156 | + | }, | |||
| 157 | + | killDaemon: func(context.Context) error { return killErr }, | |||
| 158 | + | }) | |||
| 159 | + | if !errors.Is(err, killErr) { | |||
| 160 | + | t.Fatalf("error = %v, want %v", err, killErr) | |||
| 161 | + | } | |||
| 162 | + | } | |||
| 163 | + | ||||
| 164 | + | func TestRunLocalAttachSettlesBeforeRetry(t *testing.T) { | |||
| 165 | + | var order []string | |||
| 166 | + | err := runLocalAttachWithRecovery(context.Background(), ports.IntentEphemeral, "", attachRecoveryDeps{ | |||
| 167 | + | confirmer: confirm.NewConfirmer(strings.NewReader("y\n"), &bytes.Buffer{}), | |||
| 168 | + | attach: func(context.Context, uint8, string) error { | |||
| 169 | + | order = append(order, "attach") | |||
| 170 | + | if len(order) == 1 { | |||
| 171 | + | return &client.ProtocolError{Code: ports.ErrVersionMismatch, Text: "protocol version mismatch"} | |||
| 172 | + | } | |||
| 173 | + | return nil | |||
| 174 | + | }, | |||
| 175 | + | killDaemon: func(context.Context) error { | |||
| 176 | + | order = append(order, "kill") | |||
| 177 | + | return nil | |||
| 178 | + | }, | |||
| 179 | + | settleAfterKill: func(context.Context) error { | |||
| 180 | + | order = append(order, "settle") | |||
| 181 | + | return nil | |||
| 182 | + | }, | |||
| 183 | + | }) | |||
| 184 | + | if err != nil { | |||
| 185 | + | t.Fatalf("runLocalAttachWithRecovery returned error: %v", err) | |||
| 186 | + | } | |||
| 187 | + | if got, want := strings.Join(order, ","), "attach,kill,settle,attach"; got != want { | |||
| 188 | + | t.Fatalf("order = %s, want %s", got, want) | |||
| 189 | + | } | |||
| 190 | + | } | |||
| 191 | + | ||||
| 192 | + | type errorReader struct{ err error } | |||
| 193 | + | ||||
| 194 | + | func (r errorReader) Read([]byte) (int, error) { return 0, r.err } | |||
| 195 | + | ||||
| 133 | func TestRunLocalAttachDeclineKeepsOriginalError(t *testing.T) { | 196 | func TestRunLocalAttachDeclineKeepsOriginalError(t *testing.T) { | ||
| 134 | answers := strings.NewReader("n\n") | 197 | answers := strings.NewReader("n\n") | ||
| 135 | wantErr := &client.ProtocolError{Code: ports.ErrInternal, Text: "malformed hello"} | 198 | wantErr := &client.ProtocolError{Code: ports.ErrInternal, Text: "malformed hello"} | ||
M internal/usecase/confirm/confirm.go +2 -1
| @@ -2,6 +2,7 @@ package confirm | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "bufio" | |
| 5 | + | "errors" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | "io" | |
| 7 | 8 | "strings" | |
| @@ -27,7 +28,7 @@ func (c Confirmer) Confirm(question string) (bool, error) { | |||
| 27 | 28 | } | |
| 28 | 29 | } | |
| 29 | 30 | answer, err := bufio.NewReader(c.in).ReadString('\n') | |
| 30 | - | if err != nil && err != io.EOF { | |
| 31 | + | if err != nil && !errors.Is(err, io.EOF) { | |
| 31 | 32 | return false, err | |
| 32 | 33 | } | |
| 33 | 34 | switch strings.ToLower(strings.TrimSpace(answer)) { | |
| @@ -2,6 +2,7 @@ package confirm | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bufio" | 4 | "bufio" | ||
| 5 | + | "errors" | |||
| 5 | "fmt" | 6 | "fmt" | ||
| 6 | "io" | 7 | "io" | ||
| 7 | "strings" | 8 | "strings" | ||
| @@ -27,7 +28,7 @@ func (c Confirmer) Confirm(question string) (bool, error) { | |||||
| 27 | } | 28 | } | ||
| 28 | } | 29 | } | ||
| 29 | answer, err := bufio.NewReader(c.in).ReadString('\n') | 30 | answer, err := bufio.NewReader(c.in).ReadString('\n') | ||
| 30 | - | if err != nil && err != io.EOF { | 31 | + | if err != nil && !errors.Is(err, io.EOF) { |
| 31 | return false, err | 32 | return false, err | ||
| 32 | } | 33 | } | ||
| 33 | switch strings.ToLower(strings.TrimSpace(answer)) { | 34 | switch strings.ToLower(strings.TrimSpace(answer)) { | ||
M internal/usecase/daemon/overlay.go +34 -15
| @@ -26,17 +26,17 @@ func routeOverlayBytes(data []byte, pending *[]byte, ev overlayEvents) { | |||
| 26 | 26 | for i := 0; i < len(data); { | |
| 27 | 27 | switch data[i] { | |
| 28 | 28 | case 0x1b: | |
| 29 | - | consumed, routed := routeOverlayEscape(data[i:], ev) | |
| 30 | - | if routed { | |
| 31 | - | i += consumed | |
| 32 | - | continue | |
| 33 | - | } | |
| 34 | - | if isOverlayEscapePrefix(data[i:]) { | |
| 29 | + | consumed, incomplete := routeOverlayEscape(data[i:], ev) | |
| 30 | + | if incomplete { | |
| 35 | 31 | if pending != nil { | |
| 36 | 32 | *pending = append((*pending)[:0], data[i:]...) | |
| 37 | 33 | } | |
| 38 | 34 | return | |
| 39 | 35 | } | |
| 36 | + | if consumed > 0 { | |
| 37 | + | i += consumed | |
| 38 | + | continue | |
| 39 | + | } | |
| 40 | 40 | call(ev.cancel) | |
| 41 | 41 | i++ | |
| 42 | 42 | case 0x03: | |
| @@ -74,22 +74,41 @@ func routeOverlayBytes(data []byte, pending *[]byte, ev overlayEvents) { | |||
| 74 | 74 | } | |
| 75 | 75 | } | |
| 76 | 76 | ||
| 77 | - | func routeOverlayEscape(data []byte, ev overlayEvents) (int, bool) { | |
| 78 | - | if len(data) >= 3 && (data[1] == '[' || data[1] == 'O') { | |
| 77 | + | func routeOverlayEscape(data []byte, ev overlayEvents) (consumed int, incomplete bool) { | |
| 78 | + | if len(data) < 2 { | |
| 79 | + | return 0, false | |
| 80 | + | } | |
| 81 | + | switch data[1] { | |
| 82 | + | case 'O': | |
| 83 | + | if len(data) < 3 { | |
| 84 | + | return 0, true | |
| 85 | + | } | |
| 79 | 86 | switch data[2] { | |
| 80 | 87 | case 'A': | |
| 81 | 88 | call(ev.up) | |
| 82 | - | return 3, true | |
| 83 | 89 | case 'B': | |
| 84 | 90 | call(ev.down) | |
| 85 | - | return 3, true | |
| 86 | 91 | } | |
| 92 | + | return 3, false | |
| 93 | + | case '[': | |
| 94 | + | for i := 2; i < len(data); i++ { | |
| 95 | + | if data[i] < 0x40 || data[i] > 0x7e { | |
| 96 | + | continue | |
| 97 | + | } | |
| 98 | + | if i == 2 { | |
| 99 | + | switch data[i] { | |
| 100 | + | case 'A': | |
| 101 | + | call(ev.up) | |
| 102 | + | case 'B': | |
| 103 | + | call(ev.down) | |
| 104 | + | } | |
| 105 | + | } | |
| 106 | + | return i + 1, false | |
| 107 | + | } | |
| 108 | + | return 0, true | |
| 109 | + | default: | |
| 110 | + | return 0, false | |
| 87 | 111 | } | |
| 88 | - | return 0, false | |
| 89 | - | } | |
| 90 | - | ||
| 91 | - | func isOverlayEscapePrefix(data []byte) bool { | |
| 92 | - | return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' || data[1] == 'O') | |
| 93 | 112 | } | |
| 94 | 113 | ||
| 95 | 114 | func call(fn func()) { | |
| @@ -26,17 +26,17 @@ func routeOverlayBytes(data []byte, pending *[]byte, ev overlayEvents) { | |||||
| 26 | for i := 0; i < len(data); { | 26 | for i := 0; i < len(data); { | ||
| 27 | switch data[i] { | 27 | switch data[i] { | ||
| 28 | case 0x1b: | 28 | case 0x1b: | ||
| 29 | - | consumed, routed := routeOverlayEscape(data[i:], ev) | 29 | + | consumed, incomplete := routeOverlayEscape(data[i:], ev) |
| 30 | - | if routed { | 30 | + | if incomplete { |
| 31 | - | i += consumed | |||
| 32 | - | continue | |||
| 33 | - | } | |||
| 34 | - | if isOverlayEscapePrefix(data[i:]) { | |||
| 35 | if pending != nil { | 31 | if pending != nil { | ||
| 36 | *pending = append((*pending)[:0], data[i:]...) | 32 | *pending = append((*pending)[:0], data[i:]...) | ||
| 37 | } | 33 | } | ||
| 38 | return | 34 | return | ||
| 39 | } | 35 | } | ||
| 36 | + | if consumed > 0 { | |||
| 37 | + | i += consumed | |||
| 38 | + | continue | |||
| 39 | + | } | |||
| 40 | call(ev.cancel) | 40 | call(ev.cancel) | ||
| 41 | i++ | 41 | i++ | ||
| 42 | case 0x03: | 42 | case 0x03: | ||
| @@ -74,22 +74,41 @@ func routeOverlayBytes(data []byte, pending *[]byte, ev overlayEvents) { | |||||
| 74 | } | 74 | } | ||
| 75 | } | 75 | } | ||
| 76 | 76 | ||||
| 77 | - | func routeOverlayEscape(data []byte, ev overlayEvents) (int, bool) { | 77 | + | func routeOverlayEscape(data []byte, ev overlayEvents) (consumed int, incomplete bool) { |
| 78 | - | if len(data) >= 3 && (data[1] == '[' || data[1] == 'O') { | 78 | + | if len(data) < 2 { |
| 79 | + | return 0, false | |||
| 80 | + | } | |||
| 81 | + | switch data[1] { | |||
| 82 | + | case 'O': | |||
| 83 | + | if len(data) < 3 { | |||
| 84 | + | return 0, true | |||
| 85 | + | } | |||
| 79 | switch data[2] { | 86 | switch data[2] { | ||
| 80 | case 'A': | 87 | case 'A': | ||
| 81 | call(ev.up) | 88 | call(ev.up) | ||
| 82 | - | return 3, true | |||
| 83 | case 'B': | 89 | case 'B': | ||
| 84 | call(ev.down) | 90 | call(ev.down) | ||
| 85 | - | return 3, true | |||
| 86 | } | 91 | } | ||
| 92 | + | return 3, false | |||
| 93 | + | case '[': | |||
| 94 | + | for i := 2; i < len(data); i++ { | |||
| 95 | + | if data[i] < 0x40 || data[i] > 0x7e { | |||
| 96 | + | continue | |||
| 97 | + | } | |||
| 98 | + | if i == 2 { | |||
| 99 | + | switch data[i] { | |||
| 100 | + | case 'A': | |||
| 101 | + | call(ev.up) | |||
| 102 | + | case 'B': | |||
| 103 | + | call(ev.down) | |||
| 104 | + | } | |||
| 105 | + | } | |||
| 106 | + | return i + 1, false | |||
| 107 | + | } | |||
| 108 | + | return 0, true | |||
| 109 | + | default: | |||
| 110 | + | return 0, false | |||
| 87 | } | 111 | } | ||
| 88 | - | return 0, false | |||
| 89 | - | } | |||
| 90 | - | ||||
| 91 | - | func isOverlayEscapePrefix(data []byte) bool { | |||
| 92 | - | return len(data) == 2 && data[0] == 0x1b && (data[1] == '[' || data[1] == 'O') | |||
| 93 | } | 112 | } | ||
| 94 | 113 | ||||
| 95 | func call(fn func()) { | 114 | func call(fn func()) { | ||
M internal/usecase/daemon/overlay_test.go +16 -0
| @@ -56,6 +56,22 @@ func TestRouteOverlayBytesHandlesSS3Arrows(t *testing.T) { | |||
| 56 | 56 | require.Empty(t, pending) | |
| 57 | 57 | } | |
| 58 | 58 | ||
| 59 | + | func TestRouteOverlayBytesConsumesUnhandledEscapeSequences(t *testing.T) { | |
| 60 | + | var pending []byte | |
| 61 | + | var got []rune | |
| 62 | + | var cancel int | |
| 63 | + | ev := overlayEvents{ | |
| 64 | + | rune: func(r rune) { got = append(got, r) }, | |
| 65 | + | cancel: func() { cancel++ }, | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | routeOverlayBytes([]byte("a\x1b[D\x1b[3~b\x1bOQ"), &pending, ev) | |
| 69 | + | ||
| 70 | + | require.Equal(t, []rune{'a', 'b'}, got) | |
| 71 | + | require.Equal(t, 0, cancel) | |
| 72 | + | require.Empty(t, pending) | |
| 73 | + | } | |
| 74 | + | ||
| 59 | 75 | func TestRouteOverlayBytesHandlesEditingSubmitAndCancel(t *testing.T) { | |
| 60 | 76 | var pending []byte | |
| 61 | 77 | var backspace, enter, cancel int | |
| @@ -56,6 +56,22 @@ func TestRouteOverlayBytesHandlesSS3Arrows(t *testing.T) { | |||||
| 56 | require.Empty(t, pending) | 56 | require.Empty(t, pending) | ||
| 57 | } | 57 | } | ||
| 58 | 58 | ||||
| 59 | + | func TestRouteOverlayBytesConsumesUnhandledEscapeSequences(t *testing.T) { | |||
| 60 | + | var pending []byte | |||
| 61 | + | var got []rune | |||
| 62 | + | var cancel int | |||
| 63 | + | ev := overlayEvents{ | |||
| 64 | + | rune: func(r rune) { got = append(got, r) }, | |||
| 65 | + | cancel: func() { cancel++ }, | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | routeOverlayBytes([]byte("a\x1b[D\x1b[3~b\x1bOQ"), &pending, ev) | |||
| 69 | + | ||||
| 70 | + | require.Equal(t, []rune{'a', 'b'}, got) | |||
| 71 | + | require.Equal(t, 0, cancel) | |||
| 72 | + | require.Empty(t, pending) | |||
| 73 | + | } | |||
| 74 | + | ||||
| 59 | func TestRouteOverlayBytesHandlesEditingSubmitAndCancel(t *testing.T) { | 75 | func TestRouteOverlayBytesHandlesEditingSubmitAndCancel(t *testing.T) { | ||
| 60 | var pending []byte | 76 | var pending []byte | ||
| 61 | var backspace, enter, cancel int | 77 | var backspace, enter, cancel int | ||
M internal/usecase/daemon/palette.go +6 -7
| @@ -78,7 +78,10 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte) { | |||
| 78 | 78 | ||
| 79 | 79 | if run && ok { | |
| 80 | 80 | if err := cmd.Run(paletteExec{d: d, sess: sess, ac: ac}); err != nil { | |
| 81 | - | d.log.Error("palette command failed", "err", err, "session", sess.name, "command", cmd.Code) | |
| 81 | + | sess.mu.Lock() | |
| 82 | + | name := sess.name | |
| 83 | + | sess.mu.Unlock() | |
| 84 | + | d.log.Error("palette command failed", "err", err, "session", name, "command", cmd.Code) | |
| 82 | 85 | } | |
| 83 | 86 | return | |
| 84 | 87 | } | |
| @@ -94,12 +97,8 @@ type paletteExec struct { | |||
| 94 | 97 | } | |
| 95 | 98 | ||
| 96 | 99 | func (e paletteExec) CreateTab() error { | |
| 97 | - | if err := e.d.createTab(e.sess, e.ac.size); err != nil { | |
| 98 | - | e.d.paint(e.sess, e.ac, true) | |
| 99 | - | return err | |
| 100 | - | } | |
| 101 | - | e.d.paint(e.sess, e.ac, true) | |
| 102 | - | return nil | |
| 100 | + | defer e.d.paint(e.sess, e.ac, true) | |
| 101 | + | return e.d.createTab(e.sess, e.ac.size) | |
| 103 | 102 | } | |
| 104 | 103 | ||
| 105 | 104 | func (e paletteExec) CloseTab() error { | |
| @@ -78,7 +78,10 @@ func (d *Daemon) handlePaletteInput(ac *attachedClient, data []byte) { | |||||
| 78 | 78 | ||||
| 79 | if run && ok { | 79 | if run && ok { | ||
| 80 | if err := cmd.Run(paletteExec{d: d, sess: sess, ac: ac}); err != nil { | 80 | if err := cmd.Run(paletteExec{d: d, sess: sess, ac: ac}); err != nil { | ||
| 81 | - | d.log.Error("palette command failed", "err", err, "session", sess.name, "command", cmd.Code) | 81 | + | sess.mu.Lock() |
| 82 | + | name := sess.name | |||
| 83 | + | sess.mu.Unlock() | |||
| 84 | + | d.log.Error("palette command failed", "err", err, "session", name, "command", cmd.Code) | |||
| 82 | } | 85 | } | ||
| 83 | return | 86 | return | ||
| 84 | } | 87 | } | ||
| @@ -94,12 +97,8 @@ type paletteExec struct { | |||||
| 94 | } | 97 | } | ||
| 95 | 98 | ||||
| 96 | func (e paletteExec) CreateTab() error { | 99 | func (e paletteExec) CreateTab() error { | ||
| 97 | - | if err := e.d.createTab(e.sess, e.ac.size); err != nil { | 100 | + | defer e.d.paint(e.sess, e.ac, true) |
| 98 | - | e.d.paint(e.sess, e.ac, true) | 101 | + | return e.d.createTab(e.sess, e.ac.size) |
| 99 | - | return err | |||
| 100 | - | } | |||
| 101 | - | e.d.paint(e.sess, e.ac, true) | |||
| 102 | - | return nil | |||
| 103 | } | 102 | } | ||
| 104 | 103 | ||||
| 105 | func (e paletteExec) CloseTab() error { | 104 | func (e paletteExec) CloseTab() error { | ||
M internal/usecase/daemon/render.go +2 -3
| @@ -325,9 +325,8 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 325 | 325 | } | |
| 326 | 326 | } | |
| 327 | 327 | ||
| 328 | - | // resize applies a client size change to every tab using rows-1 for the PTY | |
| 329 | - | // and VT screen, then resets the renderer shadow and repaints the composed | |
| 330 | - | // client-sized frame (including the status row). | |
| 328 | + | // desiredCursorOut computes the terminal cursor state that should be shown to | |
| 329 | + | // the client for the current tab and overlay mode. | |
| 331 | 330 | func desiredCursorOut(s *vt.Screen, copyActive bool) cursorOut { | |
| 332 | 331 | if copyActive || !s.CursorVisible() { | |
| 333 | 332 | return cursorOut{hidden: true} | |
| @@ -325,9 +325,8 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 325 | } | 325 | } | ||
| 326 | } | 326 | } | ||
| 327 | 327 | ||||
| 328 | - | // resize applies a client size change to every tab using rows-1 for the PTY | 328 | + | // desiredCursorOut computes the terminal cursor state that should be shown to |
| 329 | - | // and VT screen, then resets the renderer shadow and repaints the composed | 329 | + | // the client for the current tab and overlay mode. |
| 330 | - | // client-sized frame (including the status row). | |||
| 331 | func desiredCursorOut(s *vt.Screen, copyActive bool) cursorOut { | 330 | func desiredCursorOut(s *vt.Screen, copyActive bool) cursorOut { | ||
| 332 | if copyActive || !s.CursorVisible() { | 331 | if copyActive || !s.CursorVisible() { | ||
| 333 | return cursorOut{hidden: true} | 332 | return cursorOut{hidden: true} | ||