fix(daemon): release move picker input the client never acquired
While a move-picker interaction is open the daemon drops raw input, because a client presenting the picker consumes every key locally. A client that dropped or declined the offer therefore left input swallowed with nothing on screen. Record the offer instant on the daemon clock. Raw input inside a 2s acquisition grace is still dropped (it was in flight before the client saw the offer); raw input after it proves the client never took the interaction, so the daemon retires it, confirms PickerClosed when an effect is available, forces an authoritative repaint, and routes the input normally. Interactions are still retired when the client goes away.
- commit
- 0f340fa8c8fb019906bc9cfe344a286e58ee9335
- author
- brice · · 2026-09-22 19:44 UTC
- parent
- f65f2e3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +128 -3
M internal/usecase/daemon/input.go +3 -3
| @@ -29,7 +29,7 @@ func (d *Daemon) handleSequencedInputForAttachment(effect *attachmentEffect, _ u | |||
| 29 | 29 | ||
| 30 | 30 | func (d *Daemon) handleInput(_ *session, ac *attachedClient, data []byte) { | |
| 31 | 31 | ac.initOverlays() | |
| 32 | - | if ac.overlays.pickerClientActive() { | |
| 32 | + | if d.pickerSwallowsInput(ac, nil) { | |
| 33 | 33 | // The client-owned picker owns the input: raw keys and mouse events | |
| 34 | 34 | // must not reach the session while it is open. Only its typed | |
| 35 | 35 | // selection and close messages act, on the control path. | |
| @@ -49,9 +49,9 @@ func (d *Daemon) handleInput(_ *session, ac *attachedClient, data []byte) { | |||
| 49 | 49 | func (d *Daemon) handleInputForAttachment(effect *attachmentEffect, data []byte) { | |
| 50 | 50 | ac := effect.ac | |
| 51 | 51 | ac.initOverlays() | |
| 52 | - | if ac.overlays.pickerClientActive() { | |
| 52 | + | if d.pickerSwallowsInput(ac, effect) { | |
| 53 | 53 | // Same rule as handleInput: the client-owned picker consumes user | |
| 54 | - | // input, mouse included, until it closes. | |
| 54 | + | // input, mouse included, until it closes or is released unacquired. | |
| 55 | 55 | return | |
| 56 | 56 | } | |
| 57 | 57 | ac.mouseScan.Scan(data, | |
| @@ -29,7 +29,7 @@ func (d *Daemon) handleSequencedInputForAttachment(effect *attachmentEffect, _ u | |||||
| 29 | 29 | ||||
| 30 | func (d *Daemon) handleInput(_ *session, ac *attachedClient, data []byte) { | 30 | func (d *Daemon) handleInput(_ *session, ac *attachedClient, data []byte) { | ||
| 31 | ac.initOverlays() | 31 | ac.initOverlays() | ||
| 32 | - | if ac.overlays.pickerClientActive() { | 32 | + | if d.pickerSwallowsInput(ac, nil) { |
| 33 | // The client-owned picker owns the input: raw keys and mouse events | 33 | // The client-owned picker owns the input: raw keys and mouse events | ||
| 34 | // must not reach the session while it is open. Only its typed | 34 | // must not reach the session while it is open. Only its typed | ||
| 35 | // selection and close messages act, on the control path. | 35 | // selection and close messages act, on the control path. | ||
| @@ -49,9 +49,9 @@ func (d *Daemon) handleInput(_ *session, ac *attachedClient, data []byte) { | |||||
| 49 | func (d *Daemon) handleInputForAttachment(effect *attachmentEffect, data []byte) { | 49 | func (d *Daemon) handleInputForAttachment(effect *attachmentEffect, data []byte) { | ||
| 50 | ac := effect.ac | 50 | ac := effect.ac | ||
| 51 | ac.initOverlays() | 51 | ac.initOverlays() | ||
| 52 | - | if ac.overlays.pickerClientActive() { | 52 | + | if d.pickerSwallowsInput(ac, effect) { |
| 53 | // Same rule as handleInput: the client-owned picker consumes user | 53 | // Same rule as handleInput: the client-owned picker consumes user | ||
| 54 | - | // input, mouse included, until it closes. | 54 | + | // input, mouse included, until it closes or is released unacquired. |
| 55 | return | 55 | return | ||
| 56 | } | 56 | } | ||
| 57 | ac.mouseScan.Scan(data, | 57 | ac.mouseScan.Scan(data, | ||
M internal/usecase/daemon/overlay_runtime.go +5 -0
| @@ -49,6 +49,11 @@ type overlayRuntime struct { | |||
| 49 | 49 | pickerIntent protocol.PickerIntent | |
| 50 | 50 | pickerMoveSource moveSourceLocator | |
| 51 | 51 | pickerRequestID uint64 | |
| 52 | + | // pickerOpenedAt is the daemon-clock instant the current interaction was | |
| 53 | + | // offered. Raw input that still arrives after pickerAcquisitionGrace | |
| 54 | + | // proves the client never took the interaction, so it is released instead | |
| 55 | + | // of swallowing input forever. | |
| 56 | + | pickerOpenedAt time.Time | |
| 52 | 57 | // pickerNavigationOffers numbers the fire-and-forget navigation offers | |
| 53 | 58 | // that ask the client to open its own session picker over this live | |
| 54 | 59 | // attachment. They open no daemon interaction, so they never touch | |
| @@ -49,6 +49,11 @@ type overlayRuntime struct { | |||||
| 49 | pickerIntent protocol.PickerIntent | 49 | pickerIntent protocol.PickerIntent | ||
| 50 | pickerMoveSource moveSourceLocator | 50 | pickerMoveSource moveSourceLocator | ||
| 51 | pickerRequestID uint64 | 51 | pickerRequestID uint64 | ||
| 52 | + | // pickerOpenedAt is the daemon-clock instant the current interaction was | |||
| 53 | + | // offered. Raw input that still arrives after pickerAcquisitionGrace | |||
| 54 | + | // proves the client never took the interaction, so it is released instead | |||
| 55 | + | // of swallowing input forever. | |||
| 56 | + | pickerOpenedAt time.Time | |||
| 52 | // pickerNavigationOffers numbers the fire-and-forget navigation offers | 57 | // pickerNavigationOffers numbers the fire-and-forget navigation offers | ||
| 53 | // that ask the client to open its own session picker over this live | 58 | // that ask the client to open its own session picker over this live | ||
| 54 | // attachment. They open no daemon interaction, so they never touch | 59 | // attachment. They open no daemon interaction, so they never touch | ||
A internal/usecase/daemon/picker_failsafe_test.go +79 -0
| @@ -0,0 +1,79 @@ | |||
| 1 | + | package daemon | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "sync" | |
| 5 | + | "testing" | |
| 6 | + | "time" | |
| 7 | + | ||
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ||
| 10 | + | "github.com/bnema/vev/internal/ports" | |
| 11 | + | "github.com/bnema/vev/internal/protocol" | |
| 12 | + | ) | |
| 13 | + | ||
| 14 | + | // pickerGraceClock is a settable daemon clock. A hand-written fake is used | |
| 15 | + | // because render and attention workers read Now concurrently with the test's | |
| 16 | + | // Advance, which a mock expectation cannot express without obscuring the race | |
| 17 | + | // the mutex exists to prevent. | |
| 18 | + | type pickerGraceClock struct { | |
| 19 | + | mu sync.Mutex | |
| 20 | + | now time.Time | |
| 21 | + | } | |
| 22 | + | ||
| 23 | + | func (c *pickerGraceClock) Now() time.Time { | |
| 24 | + | c.mu.Lock() | |
| 25 | + | defer c.mu.Unlock() | |
| 26 | + | return c.now | |
| 27 | + | } | |
| 28 | + | ||
| 29 | + | func (c *pickerGraceClock) Advance(d time.Duration) { | |
| 30 | + | c.mu.Lock() | |
| 31 | + | c.now = c.now.Add(d) | |
| 32 | + | c.mu.Unlock() | |
| 33 | + | } | |
| 34 | + | ||
| 35 | + | func (*pickerGraceClock) NewTimer(time.Duration) ports.Timer { return stubTimer{} } | |
| 36 | + | ||
| 37 | + | // TestMovePickerInputFailSafe pins that raw input is never swallowed without a | |
| 38 | + | // live offer: bytes that arrive inside the acquisition grace were in flight | |
| 39 | + | // before the client saw the offer and are dropped, while raw input after the | |
| 40 | + | // grace proves the client never took the interaction and releases it. | |
| 41 | + | func TestMovePickerInputFailSafe(t *testing.T) { | |
| 42 | + | tests := []struct { | |
| 43 | + | name string | |
| 44 | + | elapsed time.Duration | |
| 45 | + | wantOpen bool | |
| 46 | + | wantReleased bool | |
| 47 | + | }{ | |
| 48 | + | {name: "in-flight input inside the grace is dropped", elapsed: 0, wantOpen: true}, | |
| 49 | + | {name: "input just before the grace is still dropped", elapsed: pickerAcquisitionGrace - time.Millisecond, wantOpen: true}, | |
| 50 | + | {name: "input after the grace releases the interaction", elapsed: pickerAcquisitionGrace, wantReleased: true}, | |
| 51 | + | } | |
| 52 | + | for _, tt := range tests { | |
| 53 | + | t.Run(tt.name, func(t *testing.T) { | |
| 54 | + | clock := &pickerGraceClock{now: time.Unix(100, 0)} | |
| 55 | + | d, source, ac, _, _, releases, sends := setupMovePickerSessionsCore(t, clock, 0) | |
| 56 | + | defer releaseAll(releases) | |
| 57 | + | ||
| 58 | + | openMovePickerForTest(t, d, ac, source, protocol.PickerIntentMovePane, moveSourceForSession(source, ac, "source-tab", "source-pane")) | |
| 59 | + | ac.overlays.pickerMu.Lock() | |
| 60 | + | interaction := ac.overlays.pickerInteraction | |
| 61 | + | ac.overlays.pickerMu.Unlock() | |
| 62 | + | clock.Advance(tt.elapsed) | |
| 63 | + | ||
| 64 | + | effect := pickActionEffectForTest(t, source, ac) | |
| 65 | + | d.handleInputForAttachment(effect, []byte("a")) | |
| 66 | + | effect.End() | |
| 67 | + | ||
| 68 | + | require.Equal(t, tt.wantOpen, ac.overlays.pickerClientActive()) | |
| 69 | + | if !tt.wantReleased { | |
| 70 | + | return | |
| 71 | + | } | |
| 72 | + | closed := awaitSentServerMessage(t, sends, "PickerClosed", func(message protocol.ServerMessage) bool { | |
| 73 | + | _, ok := message.(protocol.PickerClosed) | |
| 74 | + | return ok | |
| 75 | + | }).(protocol.PickerClosed) | |
| 76 | + | require.Equal(t, interaction, closed.InteractionID) | |
| 77 | + | }) | |
| 78 | + | } | |
| 79 | + | } | |
| @@ -0,0 +1,79 @@ | |||||
| 1 | + | package daemon | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "sync" | |||
| 5 | + | "testing" | |||
| 6 | + | "time" | |||
| 7 | + | ||||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ||||
| 10 | + | "github.com/bnema/vev/internal/ports" | |||
| 11 | + | "github.com/bnema/vev/internal/protocol" | |||
| 12 | + | ) | |||
| 13 | + | ||||
| 14 | + | // pickerGraceClock is a settable daemon clock. A hand-written fake is used | |||
| 15 | + | // because render and attention workers read Now concurrently with the test's | |||
| 16 | + | // Advance, which a mock expectation cannot express without obscuring the race | |||
| 17 | + | // the mutex exists to prevent. | |||
| 18 | + | type pickerGraceClock struct { | |||
| 19 | + | mu sync.Mutex | |||
| 20 | + | now time.Time | |||
| 21 | + | } | |||
| 22 | + | ||||
| 23 | + | func (c *pickerGraceClock) Now() time.Time { | |||
| 24 | + | c.mu.Lock() | |||
| 25 | + | defer c.mu.Unlock() | |||
| 26 | + | return c.now | |||
| 27 | + | } | |||
| 28 | + | ||||
| 29 | + | func (c *pickerGraceClock) Advance(d time.Duration) { | |||
| 30 | + | c.mu.Lock() | |||
| 31 | + | c.now = c.now.Add(d) | |||
| 32 | + | c.mu.Unlock() | |||
| 33 | + | } | |||
| 34 | + | ||||
| 35 | + | func (*pickerGraceClock) NewTimer(time.Duration) ports.Timer { return stubTimer{} } | |||
| 36 | + | ||||
| 37 | + | // TestMovePickerInputFailSafe pins that raw input is never swallowed without a | |||
| 38 | + | // live offer: bytes that arrive inside the acquisition grace were in flight | |||
| 39 | + | // before the client saw the offer and are dropped, while raw input after the | |||
| 40 | + | // grace proves the client never took the interaction and releases it. | |||
| 41 | + | func TestMovePickerInputFailSafe(t *testing.T) { | |||
| 42 | + | tests := []struct { | |||
| 43 | + | name string | |||
| 44 | + | elapsed time.Duration | |||
| 45 | + | wantOpen bool | |||
| 46 | + | wantReleased bool | |||
| 47 | + | }{ | |||
| 48 | + | {name: "in-flight input inside the grace is dropped", elapsed: 0, wantOpen: true}, | |||
| 49 | + | {name: "input just before the grace is still dropped", elapsed: pickerAcquisitionGrace - time.Millisecond, wantOpen: true}, | |||
| 50 | + | {name: "input after the grace releases the interaction", elapsed: pickerAcquisitionGrace, wantReleased: true}, | |||
| 51 | + | } | |||
| 52 | + | for _, tt := range tests { | |||
| 53 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 54 | + | clock := &pickerGraceClock{now: time.Unix(100, 0)} | |||
| 55 | + | d, source, ac, _, _, releases, sends := setupMovePickerSessionsCore(t, clock, 0) | |||
| 56 | + | defer releaseAll(releases) | |||
| 57 | + | ||||
| 58 | + | openMovePickerForTest(t, d, ac, source, protocol.PickerIntentMovePane, moveSourceForSession(source, ac, "source-tab", "source-pane")) | |||
| 59 | + | ac.overlays.pickerMu.Lock() | |||
| 60 | + | interaction := ac.overlays.pickerInteraction | |||
| 61 | + | ac.overlays.pickerMu.Unlock() | |||
| 62 | + | clock.Advance(tt.elapsed) | |||
| 63 | + | ||||
| 64 | + | effect := pickActionEffectForTest(t, source, ac) | |||
| 65 | + | d.handleInputForAttachment(effect, []byte("a")) | |||
| 66 | + | effect.End() | |||
| 67 | + | ||||
| 68 | + | require.Equal(t, tt.wantOpen, ac.overlays.pickerClientActive()) | |||
| 69 | + | if !tt.wantReleased { | |||
| 70 | + | return | |||
| 71 | + | } | |||
| 72 | + | closed := awaitSentServerMessage(t, sends, "PickerClosed", func(message protocol.ServerMessage) bool { | |||
| 73 | + | _, ok := message.(protocol.PickerClosed) | |||
| 74 | + | return ok | |||
| 75 | + | }).(protocol.PickerClosed) | |||
| 76 | + | require.Equal(t, interaction, closed.InteractionID) | |||
| 77 | + | }) | |||
| 78 | + | } | |||
| 79 | + | } | |||
M internal/usecase/daemon/picker_interaction.go +41 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "errors" | |
| 5 | 5 | "fmt" | |
| 6 | 6 | "slices" | |
| 7 | + | "time" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/vev/internal/domain" | |
| 9 | 10 | "github.com/bnema/vev/internal/protocol" | |
| @@ -21,6 +22,45 @@ const servingPickerSourceID = protocol.PickerServingSourceID | |||
| 21 | 22 | ||
| 22 | 23 | var errClientOwnedNavigation = errors.New("session navigation is client-owned") | |
| 23 | 24 | ||
| 25 | + | // pickerAcquisitionGrace bounds how long raw input from the client may still | |
| 26 | + | // arrive after a move-picker offer and be dropped as bytes that were already | |
| 27 | + | // in flight before the client saw the offer. A client that presents the picker | |
| 28 | + | // consumes every key locally, so raw input after this grace proves the client | |
| 29 | + | // never took the interaction. | |
| 30 | + | const pickerAcquisitionGrace = 2 * time.Second | |
| 31 | + | ||
| 32 | + | // pickerSwallowsInput reports whether raw input must be dropped because an | |
| 33 | + | // open client-owned picker interaction owns it. It is the fail-safe that keeps | |
| 34 | + | // input from being swallowed without a live offer: once the acquisition grace | |
| 35 | + | // elapsed, raw input releases the interaction (confirming the close to the | |
| 36 | + | // client when an effect is available and forcing an authoritative repaint) and | |
| 37 | + | // the input is routed normally. Callers must not hold pickerMu. | |
| 38 | + | func (d *Daemon) pickerSwallowsInput(ac *attachedClient, effect *attachmentEffect) bool { | |
| 39 | + | if ac == nil || ac.overlays == nil { | |
| 40 | + | return false | |
| 41 | + | } | |
| 42 | + | ac.overlays.pickerMu.Lock() | |
| 43 | + | open := ac.overlays.pickerOpen | |
| 44 | + | interaction := ac.overlays.pickerInteraction | |
| 45 | + | openedAt := ac.overlays.pickerOpenedAt | |
| 46 | + | ac.overlays.pickerMu.Unlock() | |
| 47 | + | if !open { | |
| 48 | + | return false | |
| 49 | + | } | |
| 50 | + | if d.daemonNow().Sub(openedAt) < pickerAcquisitionGrace { | |
| 51 | + | return true | |
| 52 | + | } | |
| 53 | + | if d.log != nil { | |
| 54 | + | d.log.Warn("releasing picker interaction the client never acquired", "interaction", interaction) | |
| 55 | + | } | |
| 56 | + | if d.closePickerForAttachment(ac, effect, interaction) { | |
| 57 | + | if sess := ac.currentAttachmentSession(); sess != nil { | |
| 58 | + | d.invalidateRender(sess, ac, true, "picker_interaction.go:unacquired-release") | |
| 59 | + | } | |
| 60 | + | } | |
| 61 | + | return false | |
| 62 | + | } | |
| 63 | + | ||
| 24 | 64 | // offerClientNavigationPicker asks the client to open its own session picker | |
| 25 | 65 | // over this live attachment. The daemon opens no interaction: navigation rows | |
| 26 | 66 | // come from the client's broker catalogue, the client picker consumes the | |
| @@ -130,6 +170,7 @@ func (d *Daemon) openPickerForAttachment(ac *attachedClient, effect *attachmentE | |||
| 130 | 170 | ac.overlays.pickerIntent = intent | |
| 131 | 171 | ac.overlays.pickerMoveSource = source | |
| 132 | 172 | ac.overlays.pickerRequestID = requestID | |
| 173 | + | ac.overlays.pickerOpenedAt = d.daemonNow() | |
| 133 | 174 | ac.overlays.pickerRevisions = make(map[string]uint64) | |
| 134 | 175 | ac.overlays.pickerKeys = nil | |
| 135 | 176 | ac.overlays.pickerSourcePublished = false | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | "slices" | 6 | "slices" | ||
| 7 | + | "time" | |||
| 7 | 8 | ||||
| 8 | "github.com/bnema/vev/internal/domain" | 9 | "github.com/bnema/vev/internal/domain" | ||
| 9 | "github.com/bnema/vev/internal/protocol" | 10 | "github.com/bnema/vev/internal/protocol" | ||
| @@ -21,6 +22,45 @@ const servingPickerSourceID = protocol.PickerServingSourceID | |||||
| 21 | 22 | ||||
| 22 | var errClientOwnedNavigation = errors.New("session navigation is client-owned") | 23 | var errClientOwnedNavigation = errors.New("session navigation is client-owned") | ||
| 23 | 24 | ||||
| 25 | + | // pickerAcquisitionGrace bounds how long raw input from the client may still | |||
| 26 | + | // arrive after a move-picker offer and be dropped as bytes that were already | |||
| 27 | + | // in flight before the client saw the offer. A client that presents the picker | |||
| 28 | + | // consumes every key locally, so raw input after this grace proves the client | |||
| 29 | + | // never took the interaction. | |||
| 30 | + | const pickerAcquisitionGrace = 2 * time.Second | |||
| 31 | + | ||||
| 32 | + | // pickerSwallowsInput reports whether raw input must be dropped because an | |||
| 33 | + | // open client-owned picker interaction owns it. It is the fail-safe that keeps | |||
| 34 | + | // input from being swallowed without a live offer: once the acquisition grace | |||
| 35 | + | // elapsed, raw input releases the interaction (confirming the close to the | |||
| 36 | + | // client when an effect is available and forcing an authoritative repaint) and | |||
| 37 | + | // the input is routed normally. Callers must not hold pickerMu. | |||
| 38 | + | func (d *Daemon) pickerSwallowsInput(ac *attachedClient, effect *attachmentEffect) bool { | |||
| 39 | + | if ac == nil || ac.overlays == nil { | |||
| 40 | + | return false | |||
| 41 | + | } | |||
| 42 | + | ac.overlays.pickerMu.Lock() | |||
| 43 | + | open := ac.overlays.pickerOpen | |||
| 44 | + | interaction := ac.overlays.pickerInteraction | |||
| 45 | + | openedAt := ac.overlays.pickerOpenedAt | |||
| 46 | + | ac.overlays.pickerMu.Unlock() | |||
| 47 | + | if !open { | |||
| 48 | + | return false | |||
| 49 | + | } | |||
| 50 | + | if d.daemonNow().Sub(openedAt) < pickerAcquisitionGrace { | |||
| 51 | + | return true | |||
| 52 | + | } | |||
| 53 | + | if d.log != nil { | |||
| 54 | + | d.log.Warn("releasing picker interaction the client never acquired", "interaction", interaction) | |||
| 55 | + | } | |||
| 56 | + | if d.closePickerForAttachment(ac, effect, interaction) { | |||
| 57 | + | if sess := ac.currentAttachmentSession(); sess != nil { | |||
| 58 | + | d.invalidateRender(sess, ac, true, "picker_interaction.go:unacquired-release") | |||
| 59 | + | } | |||
| 60 | + | } | |||
| 61 | + | return false | |||
| 62 | + | } | |||
| 63 | + | ||||
| 24 | // offerClientNavigationPicker asks the client to open its own session picker | 64 | // offerClientNavigationPicker asks the client to open its own session picker | ||
| 25 | // over this live attachment. The daemon opens no interaction: navigation rows | 65 | // over this live attachment. The daemon opens no interaction: navigation rows | ||
| 26 | // come from the client's broker catalogue, the client picker consumes the | 66 | // come from the client's broker catalogue, the client picker consumes the | ||
| @@ -130,6 +170,7 @@ func (d *Daemon) openPickerForAttachment(ac *attachedClient, effect *attachmentE | |||||
| 130 | ac.overlays.pickerIntent = intent | 170 | ac.overlays.pickerIntent = intent | ||
| 131 | ac.overlays.pickerMoveSource = source | 171 | ac.overlays.pickerMoveSource = source | ||
| 132 | ac.overlays.pickerRequestID = requestID | 172 | ac.overlays.pickerRequestID = requestID | ||
| 173 | + | ac.overlays.pickerOpenedAt = d.daemonNow() | |||
| 133 | ac.overlays.pickerRevisions = make(map[string]uint64) | 174 | ac.overlays.pickerRevisions = make(map[string]uint64) | ||
| 134 | ac.overlays.pickerKeys = nil | 175 | ac.overlays.pickerKeys = nil | ||
| 135 | ac.overlays.pickerSourcePublished = false | 176 | ac.overlays.pickerSourcePublished = false | ||