fix(client): keep Connecting through a broker loss and drop picker input typed while hidden
- commit
- 9d8d5774b5c63f3a13ab45e0c25dd746da2a6d58
- author
- brice · · 2026-09-24 04:23 UTC
- parent
- d1a05e5
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +61 -3
M docs/ui-driver.md +4 -2
| @@ -61,8 +61,10 @@ and it is stable for the whole run. `generation` is zero until a committed | |||
| 61 | 61 | attachment publishes one; a client must wait for `status` to become `attached` | |
| 62 | 62 | and use the generation that publication reports, and never assume | |
| 63 | 63 | `generation == 1`. A broker that is absent or incompatible is not fatal: the | |
| 64 | - | driver stays in the picker with a bounded notice and keeps answering `capture` | |
| 65 | - | and `wait`. | |
| 64 | + | driver keeps answering `capture` and `wait`. A run that requested a session | |
| 65 | + | stays `connecting` while it retries and never shows the picker first; a | |
| 66 | + | `--picker` run, a refused destination, or a failure retrying cannot fix | |
| 67 | + | presents `picker` with a bounded notice. | |
| 66 | 68 | ||
| 67 | 69 | The stream then accepts one JSON object per line. Every request has `version`, a nonzero `id`, `op`, and the discovered `attachment`: | |
| 68 | 70 | ||
| @@ -61,8 +61,10 @@ and it is stable for the whole run. `generation` is zero until a committed | |||||
| 61 | attachment publishes one; a client must wait for `status` to become `attached` | 61 | attachment publishes one; a client must wait for `status` to become `attached` | ||
| 62 | and use the generation that publication reports, and never assume | 62 | and use the generation that publication reports, and never assume | ||
| 63 | `generation == 1`. A broker that is absent or incompatible is not fatal: the | 63 | `generation == 1`. A broker that is absent or incompatible is not fatal: the | ||
| 64 | - | driver stays in the picker with a bounded notice and keeps answering `capture` | 64 | + | driver keeps answering `capture` and `wait`. A run that requested a session |
| 65 | - | and `wait`. | 65 | + | stays `connecting` while it retries and never shows the picker first; a |
| 66 | + | `--picker` run, a refused destination, or a failure retrying cannot fix | |||
| 67 | + | presents `picker` with a bounded notice. | |||
| 66 | 68 | ||||
| 67 | The stream then accepts one JSON object per line. Every request has `version`, a nonzero `id`, `op`, and the discovered `attachment`: | 69 | The stream then accepts one JSON object per line. Every request has `version`, a nonzero `id`, `op`, and the discovered `attachment`: | ||
| 68 | 70 | ||||
M internal/usecase/client/initial_navigation_test.go +45 -0
| @@ -606,3 +606,48 @@ func TestInitialNavigationWaitsForTargetObservation(t *testing.T) { | |||
| 606 | 606 | }) | |
| 607 | 607 | } | |
| 608 | 608 | } | |
| 609 | + | ||
| 610 | + | // TestInitialNavigationDropsDecisionsTakenWhileHidden pins that a selection | |
| 611 | + | // recorded by the picker while it was hidden behind the Connecting | |
| 612 | + | // presentation of a pending initial navigation never replays: after the | |
| 613 | + | // initial attachment ends, the supervisor returns to the picker and opens | |
| 614 | + | // nothing the user did not see. | |
| 615 | + | func TestInitialNavigationDropsDecisionsTakenWhileHidden(t *testing.T) { | |
| 616 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 617 | + | defer cancel() | |
| 618 | + | clock := newSupervisorTestClock() | |
| 619 | + | reader := newAttachTestReader() | |
| 620 | + | terminal := &attachTestTerminal{in: reader} | |
| 621 | + | picker := newAttachTestPicker() | |
| 622 | + | ||
| 623 | + | service := newSupervisorTestService(ports.BrokerConnectionID{1}) | |
| 624 | + | stream := newSessionTestStream() | |
| 625 | + | service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |
| 626 | + | return stream, nil | |
| 627 | + | }) | |
| 628 | + | connector := newSupervisorTestConnector(func(context.Context, int) (ports.BrokerService, error) { return service, nil }) | |
| 629 | + | // A keystroke typed during the cold start: the hidden picker recorded a | |
| 630 | + | // commit before the broker published anything. | |
| 631 | + | picker.recordOp(pickerOp{commit: true}, "row", ports.BrokerOpenStreamRequest{}) | |
| 632 | + | ||
| 633 | + | sup := mustSupervisor(t, SupervisorConfig{ | |
| 634 | + | Connector: connector, Terminal: terminal, Clock: clock, | |
| 635 | + | Jitter: func() float64 { return 0 }, Picker: picker, | |
| 636 | + | InitialNavigation: InitialNavigation{Kind: InitialNavigationCreateEphemeral, Destination: ports.BrokerEndpointFence{Local: true}}, | |
| 637 | + | }) | |
| 638 | + | done := make(chan error, 1) | |
| 639 | + | go func() { done <- sup.Run(ctx) }() | |
| 640 | + | ||
| 641 | + | require.Eventually(t, func() bool { return sup.State().Presentation == PresentConnecting }, 5*time.Second, time.Millisecond, | |
| 642 | + | "a pending navigation presents Connecting, not the picker") | |
| 643 | + | service.publishSnapshot(localDaemonSnapshot(7, 1)) | |
| 644 | + | require.Eventually(t, func() bool { return len(service.openedRequests()) == 1 }, 5*time.Second, time.Millisecond) | |
| 645 | + | ||
| 646 | + | stream.deliver(protocol.ErrorMsg{Code: protocol.ErrInternal, Text: "creation failed"}) | |
| 647 | + | require.Eventually(t, func() bool { return sup.State().Presentation == PresentPicker }, 5*time.Second, time.Millisecond) | |
| 648 | + | require.Never(t, func() bool { return len(service.openedRequests()) > 1 }, 200*time.Millisecond, 5*time.Millisecond, | |
| 649 | + | "a decision taken while hidden must not open a second stream") | |
| 650 | + | require.Zero(t, picker.resolveCount(), "the hidden commit was never resolved") | |
| 651 | + | cancel() | |
| 652 | + | require.ErrorIs(t, <-done, context.Canceled) | |
| 653 | + | } | |
| @@ -606,3 +606,48 @@ func TestInitialNavigationWaitsForTargetObservation(t *testing.T) { | |||||
| 606 | }) | 606 | }) | ||
| 607 | } | 607 | } | ||
| 608 | } | 608 | } | ||
| 609 | + | ||||
| 610 | + | // TestInitialNavigationDropsDecisionsTakenWhileHidden pins that a selection | |||
| 611 | + | // recorded by the picker while it was hidden behind the Connecting | |||
| 612 | + | // presentation of a pending initial navigation never replays: after the | |||
| 613 | + | // initial attachment ends, the supervisor returns to the picker and opens | |||
| 614 | + | // nothing the user did not see. | |||
| 615 | + | func TestInitialNavigationDropsDecisionsTakenWhileHidden(t *testing.T) { | |||
| 616 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 617 | + | defer cancel() | |||
| 618 | + | clock := newSupervisorTestClock() | |||
| 619 | + | reader := newAttachTestReader() | |||
| 620 | + | terminal := &attachTestTerminal{in: reader} | |||
| 621 | + | picker := newAttachTestPicker() | |||
| 622 | + | ||||
| 623 | + | service := newSupervisorTestService(ports.BrokerConnectionID{1}) | |||
| 624 | + | stream := newSessionTestStream() | |||
| 625 | + | service.setOpenStream(func(context.Context, ports.BrokerOpenStreamRequest) (ports.BrokerLogicalConnection, error) { | |||
| 626 | + | return stream, nil | |||
| 627 | + | }) | |||
| 628 | + | connector := newSupervisorTestConnector(func(context.Context, int) (ports.BrokerService, error) { return service, nil }) | |||
| 629 | + | // A keystroke typed during the cold start: the hidden picker recorded a | |||
| 630 | + | // commit before the broker published anything. | |||
| 631 | + | picker.recordOp(pickerOp{commit: true}, "row", ports.BrokerOpenStreamRequest{}) | |||
| 632 | + | ||||
| 633 | + | sup := mustSupervisor(t, SupervisorConfig{ | |||
| 634 | + | Connector: connector, Terminal: terminal, Clock: clock, | |||
| 635 | + | Jitter: func() float64 { return 0 }, Picker: picker, | |||
| 636 | + | InitialNavigation: InitialNavigation{Kind: InitialNavigationCreateEphemeral, Destination: ports.BrokerEndpointFence{Local: true}}, | |||
| 637 | + | }) | |||
| 638 | + | done := make(chan error, 1) | |||
| 639 | + | go func() { done <- sup.Run(ctx) }() | |||
| 640 | + | ||||
| 641 | + | require.Eventually(t, func() bool { return sup.State().Presentation == PresentConnecting }, 5*time.Second, time.Millisecond, | |||
| 642 | + | "a pending navigation presents Connecting, not the picker") | |||
| 643 | + | service.publishSnapshot(localDaemonSnapshot(7, 1)) | |||
| 644 | + | require.Eventually(t, func() bool { return len(service.openedRequests()) == 1 }, 5*time.Second, time.Millisecond) | |||
| 645 | + | ||||
| 646 | + | stream.deliver(protocol.ErrorMsg{Code: protocol.ErrInternal, Text: "creation failed"}) | |||
| 647 | + | require.Eventually(t, func() bool { return sup.State().Presentation == PresentPicker }, 5*time.Second, time.Millisecond) | |||
| 648 | + | require.Never(t, func() bool { return len(service.openedRequests()) > 1 }, 200*time.Millisecond, 5*time.Millisecond, | |||
| 649 | + | "a decision taken while hidden must not open a second stream") | |||
| 650 | + | require.Zero(t, picker.resolveCount(), "the hidden commit was never resolved") | |||
| 651 | + | cancel() | |||
| 652 | + | require.ErrorIs(t, <-done, context.Canceled) | |||
| 653 | + | } | |||
M internal/usecase/client/supervisor.go +5 -1
| @@ -658,7 +658,11 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) { | |||
| 658 | 658 | s.transition(supervisorEvent{kind: supervisorTerminal, err: termErr}) | |
| 659 | 659 | return termErr | |
| 660 | 660 | } | |
| 661 | - | s.transition(supervisorEvent{kind: supervisorNavigationSettled}) | |
| 661 | + | // A navigation left armed (the broker was lost while it waited for an | |
| 662 | + | // observation) keeps Connecting: the next attempt takes it again. | |
| 663 | + | if !s.initialNavigationPending() { | |
| 664 | + | s.transition(supervisorEvent{kind: supervisorNavigationSettled}) | |
| 665 | + | } | |
| 662 | 666 | ||
| 663 | 667 | // Ready phase: fold publications, admit committed attachments one at a | |
| 664 | 668 | // time, and return to the picker after each. A committed attachment | |
| @@ -658,7 +658,11 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) { | |||||
| 658 | s.transition(supervisorEvent{kind: supervisorTerminal, err: termErr}) | 658 | s.transition(supervisorEvent{kind: supervisorTerminal, err: termErr}) | ||
| 659 | return termErr | 659 | return termErr | ||
| 660 | } | 660 | } | ||
| 661 | - | s.transition(supervisorEvent{kind: supervisorNavigationSettled}) | 661 | + | // A navigation left armed (the broker was lost while it waited for an |
| 662 | + | // observation) keeps Connecting: the next attempt takes it again. | |||
| 663 | + | if !s.initialNavigationPending() { | |||
| 664 | + | s.transition(supervisorEvent{kind: supervisorNavigationSettled}) | |||
| 665 | + | } | |||
| 662 | 666 | ||||
| 663 | // Ready phase: fold publications, admit committed attachments one at a | 667 | // Ready phase: fold publications, admit committed attachments one at a | ||
| 664 | // time, and return to the picker after each. A committed attachment | 668 | // time, and return to the picker after each. A committed attachment | ||
M internal/usecase/client/supervisor_attach.go +7 -0
| @@ -507,6 +507,13 @@ func (s *Supervisor) runInitialNavigation(ctx context.Context, input *terminalIn | |||
| 507 | 507 | s.reportAttachmentFailure(err) | |
| 508 | 508 | return false, nil | |
| 509 | 509 | } | |
| 510 | + | // The picker was hidden behind Connecting while this navigation was | |
| 511 | + | // pending: keys typed then must not replay as a selection or a close | |
| 512 | + | // after the attachment returns to it. | |
| 513 | + | if s.cfg.Picker != nil { | |
| 514 | + | s.cfg.Picker.TakeOp() | |
| 515 | + | } | |
| 516 | + | s.pendingPickerKey = "" | |
| 510 | 517 | return s.runResolvedAttachment(ctx, input, service, pickerAttachmentTarget{request: request}, SessionEnvironmentLocalCLI) | |
| 511 | 518 | } | |
| 512 | 519 | ||
| @@ -507,6 +507,13 @@ func (s *Supervisor) runInitialNavigation(ctx context.Context, input *terminalIn | |||||
| 507 | s.reportAttachmentFailure(err) | 507 | s.reportAttachmentFailure(err) | ||
| 508 | return false, nil | 508 | return false, nil | ||
| 509 | } | 509 | } | ||
| 510 | + | // The picker was hidden behind Connecting while this navigation was | |||
| 511 | + | // pending: keys typed then must not replay as a selection or a close | |||
| 512 | + | // after the attachment returns to it. | |||
| 513 | + | if s.cfg.Picker != nil { | |||
| 514 | + | s.cfg.Picker.TakeOp() | |||
| 515 | + | } | |||
| 516 | + | s.pendingPickerKey = "" | |||
| 510 | return s.runResolvedAttachment(ctx, input, service, pickerAttachmentTarget{request: request}, SessionEnvironmentLocalCLI) | 517 | return s.runResolvedAttachment(ctx, input, service, pickerAttachmentTarget{request: request}, SessionEnvironmentLocalCLI) | ||
| 511 | } | 518 | } | ||
| 512 | 519 | ||||