refactor(usecase/client): delete dead picker and input pump helpers
attachmentAuthority.currentToken, pickerConsumer.setDrain,
pickerLoop.cursorKey, pickerRenderer.reset, and
terminalInputPump.claim/hasExited/resume have no production caller left.
Tests still exercising real behavior through the deleted seams are rewritten
onto the production path:
- currentToken's four callers now read authority.foreground().Token()
through a new authorityToken test helper.
- the six terminalInputPump.claim callers now call tryClaim directly and
assert success, exactly as production's own callers do.
pickerCatalogueErrorIs is heavily used across four test files to classify a
still-live error type; it moves into picker_catalogue_test.go rather than
being deleted. pickerRenderer.reset's dedicated test is deleted with it: its
narrower behavior (clearing prevBounds without invalidating the shadow) has
no production caller, superseded by invalidate() everywhere reset used to be
called.- commit
- c45bf3b42b3c6dbda5862ac214d22a47d9712332
- author
- brice · · 2026-09-23 01:55 UTC
- parent
- e120b49
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
12 files · +38 -114
M internal/usecase/client/attachment_worker.go +0 -12
| @@ -214,18 +214,6 @@ type attachmentAuthority struct { | |||
| 214 | 214 | state attachmentAuthorityState | |
| 215 | 215 | } | |
| 216 | 216 | ||
| 217 | - | func (a *attachmentAuthority) currentToken() AttachmentToken { | |
| 218 | - | if a == nil { | |
| 219 | - | return AttachmentToken{} | |
| 220 | - | } | |
| 221 | - | a.mu.Lock() | |
| 222 | - | defer a.mu.Unlock() | |
| 223 | - | if a.current == nil { | |
| 224 | - | return AttachmentToken{} | |
| 225 | - | } | |
| 226 | - | return a.current.token | |
| 227 | - | } | |
| 228 | - | ||
| 229 | 217 | // grant installs fg as the sole foreground in the active state. It fails while | |
| 230 | 218 | // another grant is live or while the shared input pump already has a consumer, | |
| 231 | 219 | // which is what keeps exactly one foreground owner. | |
| @@ -214,18 +214,6 @@ type attachmentAuthority struct { | |||||
| 214 | state attachmentAuthorityState | 214 | state attachmentAuthorityState | ||
| 215 | } | 215 | } | ||
| 216 | 216 | ||||
| 217 | - | func (a *attachmentAuthority) currentToken() AttachmentToken { | |||
| 218 | - | if a == nil { | |||
| 219 | - | return AttachmentToken{} | |||
| 220 | - | } | |||
| 221 | - | a.mu.Lock() | |||
| 222 | - | defer a.mu.Unlock() | |||
| 223 | - | if a.current == nil { | |||
| 224 | - | return AttachmentToken{} | |||
| 225 | - | } | |||
| 226 | - | return a.current.token | |||
| 227 | - | } | |||
| 228 | - | ||||
| 229 | // grant installs fg as the sole foreground in the active state. It fails while | 217 | // grant installs fg as the sole foreground in the active state. It fails while | ||
| 230 | // another grant is live or while the shared input pump already has a consumer, | 218 | // another grant is live or while the shared input pump already has a consumer, | ||
| 231 | // which is what keeps exactly one foreground owner. | 219 | // which is what keeps exactly one foreground owner. | ||
M internal/usecase/client/attachment_worker_test.go +16 -2
| @@ -17,6 +17,18 @@ import ( | |||
| 17 | 17 | "github.com/bnema/vev/internal/protocol" | |
| 18 | 18 | ) | |
| 19 | 19 | ||
| 20 | + | // authorityToken reads the foreground token an attachmentAuthority currently | |
| 21 | + | // grants, or the zero token when nothing holds the slot. It exercises the same | |
| 22 | + | // foreground() accessor the supervisor uses, so tests observe the production | |
| 23 | + | // path instead of a test-only accessor. | |
| 24 | + | func authorityToken(a *attachmentAuthority) AttachmentToken { | |
| 25 | + | fg := a.foreground() | |
| 26 | + | if fg == nil { | |
| 27 | + | return AttachmentToken{} | |
| 28 | + | } | |
| 29 | + | return fg.Token() | |
| 30 | + | } | |
| 31 | + | ||
| 20 | 32 | // workerTestStream is one scripted broker logical stream. It records typed | |
| 21 | 33 | // client messages and unblocks ReceiveServer when the supervisor closes it, | |
| 22 | 34 | // exactly like a real logical stream's Close contract. | |
| @@ -689,7 +701,8 @@ func TestAttachmentWorkerPumpClaimAndCancelRace(t *testing.T) { | |||
| 689 | 701 | pump.start() | |
| 690 | 702 | defer pump.stop() | |
| 691 | 703 | host := newWorkerTestHost(newWorkerTestTerminal(), pump, nil) | |
| 692 | - | rival := pump.claim() | |
| 704 | + | rival, claimed := pump.tryClaim() | |
| 705 | + | require.True(t, claimed) | |
| 693 | 706 | _, ok := host.Begin(context.Background(), AttachmentToken{1, 1}, blockingWorker(), newWorkerTestStream()) | |
| 694 | 707 | require.False(t, ok) | |
| 695 | 708 | require.Nil(t, host.authority.foreground()) | |
| @@ -879,7 +892,8 @@ func TestAttachmentWorkerBeginRefusalsRetainStream(t *testing.T) { | |||
| 879 | 892 | t.Run("rival consumer owns input", func(t *testing.T) { | |
| 880 | 893 | pump := newTerminalInputPump(nil) | |
| 881 | 894 | host := newWorkerTestHost(newWorkerTestTerminal(), pump, nil) | |
| 882 | - | rival := pump.claim() | |
| 895 | + | rival, claimed := pump.tryClaim() | |
| 896 | + | require.True(t, claimed) | |
| 883 | 897 | defer pump.revoke(rival) | |
| 884 | 898 | stream := newWorkerTestStream() | |
| 885 | 899 | _, ok := host.Begin(context.Background(), AttachmentToken{1, 1}, blockingWorker(), stream) | |
| @@ -17,6 +17,18 @@ import ( | |||||
| 17 | "github.com/bnema/vev/internal/protocol" | 17 | "github.com/bnema/vev/internal/protocol" | ||
| 18 | ) | 18 | ) | ||
| 19 | 19 | ||||
| 20 | + | // authorityToken reads the foreground token an attachmentAuthority currently | |||
| 21 | + | // grants, or the zero token when nothing holds the slot. It exercises the same | |||
| 22 | + | // foreground() accessor the supervisor uses, so tests observe the production | |||
| 23 | + | // path instead of a test-only accessor. | |||
| 24 | + | func authorityToken(a *attachmentAuthority) AttachmentToken { | |||
| 25 | + | fg := a.foreground() | |||
| 26 | + | if fg == nil { | |||
| 27 | + | return AttachmentToken{} | |||
| 28 | + | } | |||
| 29 | + | return fg.Token() | |||
| 30 | + | } | |||
| 31 | + | ||||
| 20 | // workerTestStream is one scripted broker logical stream. It records typed | 32 | // workerTestStream is one scripted broker logical stream. It records typed | ||
| 21 | // client messages and unblocks ReceiveServer when the supervisor closes it, | 33 | // client messages and unblocks ReceiveServer when the supervisor closes it, | ||
| 22 | // exactly like a real logical stream's Close contract. | 34 | // exactly like a real logical stream's Close contract. | ||
| @@ -689,7 +701,8 @@ func TestAttachmentWorkerPumpClaimAndCancelRace(t *testing.T) { | |||||
| 689 | pump.start() | 701 | pump.start() | ||
| 690 | defer pump.stop() | 702 | defer pump.stop() | ||
| 691 | host := newWorkerTestHost(newWorkerTestTerminal(), pump, nil) | 703 | host := newWorkerTestHost(newWorkerTestTerminal(), pump, nil) | ||
| 692 | - | rival := pump.claim() | 704 | + | rival, claimed := pump.tryClaim() |
| 705 | + | require.True(t, claimed) | |||
| 693 | _, ok := host.Begin(context.Background(), AttachmentToken{1, 1}, blockingWorker(), newWorkerTestStream()) | 706 | _, ok := host.Begin(context.Background(), AttachmentToken{1, 1}, blockingWorker(), newWorkerTestStream()) | ||
| 694 | require.False(t, ok) | 707 | require.False(t, ok) | ||
| 695 | require.Nil(t, host.authority.foreground()) | 708 | require.Nil(t, host.authority.foreground()) | ||
| @@ -879,7 +892,8 @@ func TestAttachmentWorkerBeginRefusalsRetainStream(t *testing.T) { | |||||
| 879 | t.Run("rival consumer owns input", func(t *testing.T) { | 892 | t.Run("rival consumer owns input", func(t *testing.T) { | ||
| 880 | pump := newTerminalInputPump(nil) | 893 | pump := newTerminalInputPump(nil) | ||
| 881 | host := newWorkerTestHost(newWorkerTestTerminal(), pump, nil) | 894 | host := newWorkerTestHost(newWorkerTestTerminal(), pump, nil) | ||
| 882 | - | rival := pump.claim() | 895 | + | rival, claimed := pump.tryClaim() |
| 896 | + | require.True(t, claimed) | |||
| 883 | defer pump.revoke(rival) | 897 | defer pump.revoke(rival) | ||
| 884 | stream := newWorkerTestStream() | 898 | stream := newWorkerTestStream() | ||
| 885 | _, ok := host.Begin(context.Background(), AttachmentToken{1, 1}, blockingWorker(), stream) | 899 | _, ok := host.Begin(context.Background(), AttachmentToken{1, 1}, blockingWorker(), stream) | ||
M internal/usecase/client/lifecycle_p54a_test.go +7 -4
| @@ -40,7 +40,7 @@ func TestP54LifecycleActionsAreDistinct(t *testing.T) { | |||
| 40 | 40 | awaitStreamHello(t, &sync.Mutex{}, &stream) | |
| 41 | 41 | deliverReadyStream(t, stream) | |
| 42 | 42 | awaitAttachedState(t, harness.sup) | |
| 43 | - | actions <- AttachmentLifecycleAction{Token: harness.sup.attachments.authority.currentToken(), Kind: tc.action} | |
| 43 | + | actions <- AttachmentLifecycleAction{Token: authorityToken(&harness.sup.attachments.authority), Kind: tc.action} | |
| 44 | 44 | require.Equal(t, tc.wantNotice, (<-notices).Kind) | |
| 45 | 45 | if tc.wantExit { | |
| 46 | 46 | require.NoError(t, harness.waitRun(t)) | |
| @@ -257,7 +257,8 @@ func TestP54BrokerRestartDoesNotReattachOrReplayUnknownMutation(t *testing.T) { | |||
| 257 | 257 | func TestP54ReadyMapPrunedAcrossClaims(t *testing.T) { | |
| 258 | 258 | pump := newTerminalInputPump(nil) | |
| 259 | 259 | for i := 0; i < 100; i++ { | |
| 260 | - | consumer := pump.claim() | |
| 260 | + | consumer, ok := pump.tryClaim() | |
| 261 | + | require.True(t, ok) | |
| 261 | 262 | pump.revoke(consumer) | |
| 262 | 263 | } | |
| 263 | 264 | pump.readyMu.Lock() | |
| @@ -285,7 +286,8 @@ func TestP54PickerCommittedReadIsNotReplayedToAttachment(t *testing.T) { | |||
| 285 | 286 | }, 5*time.Second, time.Millisecond) | |
| 286 | 287 | require.Equal(t, []byte("1\r"), committed) | |
| 287 | 288 | lifetime.releasePicker() | |
| 288 | - | attachment := lifetime.pump.claim() | |
| 289 | + | attachment, claimed := lifetime.pump.tryClaim() | |
| 290 | + | require.True(t, claimed) | |
| 289 | 291 | _, ok := lifetime.pump.take(context.Background(), attachment) | |
| 290 | 292 | require.False(t, ok, "committed picker read must not cross the handoff") | |
| 291 | 293 | lifetime.pump.revoke(attachment) | |
| @@ -346,7 +348,8 @@ func TestP54UndecidedAttachmentReadIsDroppedBeforePicker(t *testing.T) { | |||
| 346 | 348 | lifetime := startTerminalInputLifetime(reader, consumer) | |
| 347 | 349 | defer lifetime.stop() | |
| 348 | 350 | lifetime.releasePicker() | |
| 349 | - | attachment := lifetime.pump.claim() | |
| 351 | + | attachment, claimed := lifetime.pump.tryClaim() | |
| 352 | + | require.True(t, claimed) | |
| 350 | 353 | reader.push([]byte("ls\r")) | |
| 351 | 354 | var delivery terminalReadResult | |
| 352 | 355 | require.Eventually(t, func() bool { | |
| @@ -40,7 +40,7 @@ func TestP54LifecycleActionsAreDistinct(t *testing.T) { | |||||
| 40 | awaitStreamHello(t, &sync.Mutex{}, &stream) | 40 | awaitStreamHello(t, &sync.Mutex{}, &stream) | ||
| 41 | deliverReadyStream(t, stream) | 41 | deliverReadyStream(t, stream) | ||
| 42 | awaitAttachedState(t, harness.sup) | 42 | awaitAttachedState(t, harness.sup) | ||
| 43 | - | actions <- AttachmentLifecycleAction{Token: harness.sup.attachments.authority.currentToken(), Kind: tc.action} | 43 | + | actions <- AttachmentLifecycleAction{Token: authorityToken(&harness.sup.attachments.authority), Kind: tc.action} |
| 44 | require.Equal(t, tc.wantNotice, (<-notices).Kind) | 44 | require.Equal(t, tc.wantNotice, (<-notices).Kind) | ||
| 45 | if tc.wantExit { | 45 | if tc.wantExit { | ||
| 46 | require.NoError(t, harness.waitRun(t)) | 46 | require.NoError(t, harness.waitRun(t)) | ||
| @@ -257,7 +257,8 @@ func TestP54BrokerRestartDoesNotReattachOrReplayUnknownMutation(t *testing.T) { | |||||
| 257 | func TestP54ReadyMapPrunedAcrossClaims(t *testing.T) { | 257 | func TestP54ReadyMapPrunedAcrossClaims(t *testing.T) { | ||
| 258 | pump := newTerminalInputPump(nil) | 258 | pump := newTerminalInputPump(nil) | ||
| 259 | for i := 0; i < 100; i++ { | 259 | for i := 0; i < 100; i++ { | ||
| 260 | - | consumer := pump.claim() | 260 | + | consumer, ok := pump.tryClaim() |
| 261 | + | require.True(t, ok) | |||
| 261 | pump.revoke(consumer) | 262 | pump.revoke(consumer) | ||
| 262 | } | 263 | } | ||
| 263 | pump.readyMu.Lock() | 264 | pump.readyMu.Lock() | ||
| @@ -285,7 +286,8 @@ func TestP54PickerCommittedReadIsNotReplayedToAttachment(t *testing.T) { | |||||
| 285 | }, 5*time.Second, time.Millisecond) | 286 | }, 5*time.Second, time.Millisecond) | ||
| 286 | require.Equal(t, []byte("1\r"), committed) | 287 | require.Equal(t, []byte("1\r"), committed) | ||
| 287 | lifetime.releasePicker() | 288 | lifetime.releasePicker() | ||
| 288 | - | attachment := lifetime.pump.claim() | 289 | + | attachment, claimed := lifetime.pump.tryClaim() |
| 290 | + | require.True(t, claimed) | |||
| 289 | _, ok := lifetime.pump.take(context.Background(), attachment) | 291 | _, ok := lifetime.pump.take(context.Background(), attachment) | ||
| 290 | require.False(t, ok, "committed picker read must not cross the handoff") | 292 | require.False(t, ok, "committed picker read must not cross the handoff") | ||
| 291 | lifetime.pump.revoke(attachment) | 293 | lifetime.pump.revoke(attachment) | ||
| @@ -346,7 +348,8 @@ func TestP54UndecidedAttachmentReadIsDroppedBeforePicker(t *testing.T) { | |||||
| 346 | lifetime := startTerminalInputLifetime(reader, consumer) | 348 | lifetime := startTerminalInputLifetime(reader, consumer) | ||
| 347 | defer lifetime.stop() | 349 | defer lifetime.stop() | ||
| 348 | lifetime.releasePicker() | 350 | lifetime.releasePicker() | ||
| 349 | - | attachment := lifetime.pump.claim() | 351 | + | attachment, claimed := lifetime.pump.tryClaim() |
| 352 | + | require.True(t, claimed) | |||
| 350 | reader.push([]byte("ls\r")) | 353 | reader.push([]byte("ls\r")) | ||
| 351 | var delivery terminalReadResult | 354 | var delivery terminalReadResult | ||
| 352 | require.Eventually(t, func() bool { | 355 | require.Eventually(t, func() bool { | ||
M internal/usecase/client/picker_catalogue.go +0 -8
| @@ -4,7 +4,6 @@ import ( | |||
| 4 | 4 | "crypto/sha256" | |
| 5 | 5 | "encoding/base64" | |
| 6 | 6 | "encoding/binary" | |
| 7 | - | "errors" | |
| 8 | 7 | "fmt" | |
| 9 | 8 | "strings" | |
| 10 | 9 | "sync" | |
| @@ -126,13 +125,6 @@ func (e pickerCatalogueError) Error() string { | |||
| 126 | 125 | return "vev: picker selection " + e.Code.String() + ": " + e.Text | |
| 127 | 126 | } | |
| 128 | 127 | ||
| 129 | - | // pickerCatalogueErrorIs reports whether err is a typed catalogue refusal with | |
| 130 | - | // the supplied code, so callers classify without matching message text. | |
| 131 | - | func pickerCatalogueErrorIs(err error, code pickerCatalogueErrorCode) bool { | |
| 132 | - | var typed pickerCatalogueError | |
| 133 | - | return errors.As(err, &typed) && typed.Code == code | |
| 134 | - | } | |
| 135 | - | ||
| 136 | 128 | // pickerSelectionKind names one closed admission variant a resolved selection | |
| 137 | 129 | // can carry. | |
| 138 | 130 | type pickerSelectionKind uint8 | |
| @@ -4,7 +4,6 @@ import ( | |||||
| 4 | "crypto/sha256" | 4 | "crypto/sha256" | ||
| 5 | "encoding/base64" | 5 | "encoding/base64" | ||
| 6 | "encoding/binary" | 6 | "encoding/binary" | ||
| 7 | - | "errors" | |||
| 8 | "fmt" | 7 | "fmt" | ||
| 9 | "strings" | 8 | "strings" | ||
| 10 | "sync" | 9 | "sync" | ||
| @@ -126,13 +125,6 @@ func (e pickerCatalogueError) Error() string { | |||||
| 126 | return "vev: picker selection " + e.Code.String() + ": " + e.Text | 125 | return "vev: picker selection " + e.Code.String() + ": " + e.Text | ||
| 127 | } | 126 | } | ||
| 128 | 127 | ||||
| 129 | - | // pickerCatalogueErrorIs reports whether err is a typed catalogue refusal with | |||
| 130 | - | // the supplied code, so callers classify without matching message text. | |||
| 131 | - | func pickerCatalogueErrorIs(err error, code pickerCatalogueErrorCode) bool { | |||
| 132 | - | var typed pickerCatalogueError | |||
| 133 | - | return errors.As(err, &typed) && typed.Code == code | |||
| 134 | - | } | |||
| 135 | - | ||||
| 136 | // pickerSelectionKind names one closed admission variant a resolved selection | 128 | // pickerSelectionKind names one closed admission variant a resolved selection | ||
| 137 | // can carry. | 129 | // can carry. | ||
| 138 | type pickerSelectionKind uint8 | 130 | type pickerSelectionKind uint8 | ||
M internal/usecase/client/picker_catalogue_test.go +9 -0
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package client | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "errors" | |
| 4 | 5 | "fmt" | |
| 5 | 6 | "sync" | |
| 6 | 7 | "testing" | |
| @@ -20,6 +21,14 @@ import ( | |||
| 20 | 21 | ||
| 21 | 22 | const pickerTestFreshness = 30 * time.Second | |
| 22 | 23 | ||
| 24 | + | // pickerCatalogueErrorIs reports whether err is a typed catalogue refusal with | |
| 25 | + | // the supplied code, so tests classify a resolution failure without matching | |
| 26 | + | // message text. | |
| 27 | + | func pickerCatalogueErrorIs(err error, code pickerCatalogueErrorCode) bool { | |
| 28 | + | var typed pickerCatalogueError | |
| 29 | + | return errors.As(err, &typed) && typed.Code == code | |
| 30 | + | } | |
| 31 | + | ||
| 23 | 32 | func pickerTestPolicy() ports.BrokerPolicy { | |
| 24 | 33 | return ports.BrokerPolicy{ | |
| 25 | 34 | ProtocolVersion: protocol.Version, | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package client | 1 | package client | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "errors" | |||
| 4 | "fmt" | 5 | "fmt" | ||
| 5 | "sync" | 6 | "sync" | ||
| 6 | "testing" | 7 | "testing" | ||
| @@ -20,6 +21,14 @@ import ( | |||||
| 20 | 21 | ||||
| 21 | const pickerTestFreshness = 30 * time.Second | 22 | const pickerTestFreshness = 30 * time.Second | ||
| 22 | 23 | ||||
| 24 | + | // pickerCatalogueErrorIs reports whether err is a typed catalogue refusal with | |||
| 25 | + | // the supplied code, so tests classify a resolution failure without matching | |||
| 26 | + | // message text. | |||
| 27 | + | func pickerCatalogueErrorIs(err error, code pickerCatalogueErrorCode) bool { | |||
| 28 | + | var typed pickerCatalogueError | |||
| 29 | + | return errors.As(err, &typed) && typed.Code == code | |||
| 30 | + | } | |||
| 31 | + | ||||
| 23 | func pickerTestPolicy() ports.BrokerPolicy { | 32 | func pickerTestPolicy() ports.BrokerPolicy { | ||
| 24 | return ports.BrokerPolicy{ | 33 | return ports.BrokerPolicy{ | ||
| 25 | ProtocolVersion: protocol.Version, | 34 | ProtocolVersion: protocol.Version, | ||
M internal/usecase/client/picker_input.go +0 -13
| @@ -84,19 +84,6 @@ func (c *pickerConsumer) setOwned(interaction, generation uint64) { | |||
| 84 | 84 | c.mu.Unlock() | |
| 85 | 85 | } | |
| 86 | 86 | ||
| 87 | - | // setDrain publishes the release window: the interaction is retired, its | |
| 88 | - | // repaint is in flight, and every batch must be consumed and dropped so a | |
| 89 | - | // picker-bound key can never reach the session. | |
| 90 | - | func (c *pickerConsumer) setDrain(interaction, generation uint64) { | |
| 91 | - | if c == nil { | |
| 92 | - | return | |
| 93 | - | } | |
| 94 | - | c.mu.Lock() | |
| 95 | - | c.state = pickerInputState{interaction: interaction, generation: generation, drain: true} | |
| 96 | - | c.resetDecoderLocked() | |
| 97 | - | c.mu.Unlock() | |
| 98 | - | } | |
| 99 | - | ||
| 100 | 87 | // clear releases input back to the session and purges the decoder state. | |
| 101 | 88 | func (c *pickerConsumer) clear() { | |
| 102 | 89 | if c == nil { | |
| @@ -84,19 +84,6 @@ func (c *pickerConsumer) setOwned(interaction, generation uint64) { | |||||
| 84 | c.mu.Unlock() | 84 | c.mu.Unlock() | ||
| 85 | } | 85 | } | ||
| 86 | 86 | ||||
| 87 | - | // setDrain publishes the release window: the interaction is retired, its | |||
| 88 | - | // repaint is in flight, and every batch must be consumed and dropped so a | |||
| 89 | - | // picker-bound key can never reach the session. | |||
| 90 | - | func (c *pickerConsumer) setDrain(interaction, generation uint64) { | |||
| 91 | - | if c == nil { | |||
| 92 | - | return | |||
| 93 | - | } | |||
| 94 | - | c.mu.Lock() | |||
| 95 | - | c.state = pickerInputState{interaction: interaction, generation: generation, drain: true} | |||
| 96 | - | c.resetDecoderLocked() | |||
| 97 | - | c.mu.Unlock() | |||
| 98 | - | } | |||
| 99 | - | ||||
| 100 | // clear releases input back to the session and purges the decoder state. | 87 | // clear releases input back to the session and purges the decoder state. | ||
| 101 | func (c *pickerConsumer) clear() { | 88 | func (c *pickerConsumer) clear() { | ||
| 102 | if c == nil { | 89 | if c == nil { | ||
M internal/usecase/client/picker_interaction.go +0 -26
| @@ -306,23 +306,6 @@ func pickerPreviewSize(terminal domain.Size) domain.Size { | |||
| 306 | 306 | // emptyPickerPreview is the zero preview used until the daemon publishes one. | |
| 307 | 307 | func emptyPickerPreview() picker.Preview { return picker.Preview{} } | |
| 308 | 308 | ||
| 309 | - | // cursorKey names the row the modal currently displays. The daemon revalidates | |
| 310 | - | // it against the interaction it published, so it stays opaque here. | |
| 311 | - | func (l *pickerLoop) cursorKey() string { | |
| 312 | - | if l == nil || l.model == nil { | |
| 313 | - | return "" | |
| 314 | - | } | |
| 315 | - | line, ok := l.model.Selected() | |
| 316 | - | if !ok { | |
| 317 | - | return "" | |
| 318 | - | } | |
| 319 | - | identity, ok := l.rows[line.Key] | |
| 320 | - | if !ok { | |
| 321 | - | return "" | |
| 322 | - | } | |
| 323 | - | return identity.key | |
| 324 | - | } | |
| 325 | - | ||
| 326 | 309 | // clampPreviewDimension bounds one requested viewport dimension. | |
| 327 | 310 | func clampPreviewDimension(value int, maxValue uint16) uint16 { | |
| 328 | 311 | if value <= 0 { | |
| @@ -424,15 +407,6 @@ func (r *pickerRenderer) render(loop *pickerLoop, size domain.Size, preview pick | |||
| 424 | 407 | return data | |
| 425 | 408 | } | |
| 426 | 409 | ||
| 427 | - | // reset forgets modal damage after the daemon has restored the authoritative | |
| 428 | - | // session frame. A later picker must not clear cells from an earlier lease. | |
| 429 | - | func (r *pickerRenderer) reset() { | |
| 430 | - | if r == nil { | |
| 431 | - | return | |
| 432 | - | } | |
| 433 | - | r.prevBounds = nil | |
| 434 | - | } | |
| 435 | - | ||
| 436 | 410 | // invalidate forgets everything the renderer believes is on screen. The next | |
| 437 | 411 | // render re-primes an empty shadow and writes the whole box again: someone | |
| 438 | 412 | // else (an attachment's output or its authoritative repaint) owned the | |
| @@ -306,23 +306,6 @@ func pickerPreviewSize(terminal domain.Size) domain.Size { | |||||
| 306 | // emptyPickerPreview is the zero preview used until the daemon publishes one. | 306 | // emptyPickerPreview is the zero preview used until the daemon publishes one. | ||
| 307 | func emptyPickerPreview() picker.Preview { return picker.Preview{} } | 307 | func emptyPickerPreview() picker.Preview { return picker.Preview{} } | ||
| 308 | 308 | ||||
| 309 | - | // cursorKey names the row the modal currently displays. The daemon revalidates | |||
| 310 | - | // it against the interaction it published, so it stays opaque here. | |||
| 311 | - | func (l *pickerLoop) cursorKey() string { | |||
| 312 | - | if l == nil || l.model == nil { | |||
| 313 | - | return "" | |||
| 314 | - | } | |||
| 315 | - | line, ok := l.model.Selected() | |||
| 316 | - | if !ok { | |||
| 317 | - | return "" | |||
| 318 | - | } | |||
| 319 | - | identity, ok := l.rows[line.Key] | |||
| 320 | - | if !ok { | |||
| 321 | - | return "" | |||
| 322 | - | } | |||
| 323 | - | return identity.key | |||
| 324 | - | } | |||
| 325 | - | ||||
| 326 | // clampPreviewDimension bounds one requested viewport dimension. | 309 | // clampPreviewDimension bounds one requested viewport dimension. | ||
| 327 | func clampPreviewDimension(value int, maxValue uint16) uint16 { | 310 | func clampPreviewDimension(value int, maxValue uint16) uint16 { | ||
| 328 | if value <= 0 { | 311 | if value <= 0 { | ||
| @@ -424,15 +407,6 @@ func (r *pickerRenderer) render(loop *pickerLoop, size domain.Size, preview pick | |||||
| 424 | return data | 407 | return data | ||
| 425 | } | 408 | } | ||
| 426 | 409 | ||||
| 427 | - | // reset forgets modal damage after the daemon has restored the authoritative | |||
| 428 | - | // session frame. A later picker must not clear cells from an earlier lease. | |||
| 429 | - | func (r *pickerRenderer) reset() { | |||
| 430 | - | if r == nil { | |||
| 431 | - | return | |||
| 432 | - | } | |||
| 433 | - | r.prevBounds = nil | |||
| 434 | - | } | |||
| 435 | - | ||||
| 436 | // invalidate forgets everything the renderer believes is on screen. The next | 410 | // invalidate forgets everything the renderer believes is on screen. The next | ||
| 437 | // render re-primes an empty shadow and writes the whole box again: someone | 411 | // render re-primes an empty shadow and writes the whole box again: someone | ||
| 438 | // else (an attachment's output or its authoritative repaint) owned the | 412 | // else (an attachment's output or its authoritative repaint) owned the | ||
M internal/usecase/client/picker_interaction_test.go +0 -15
| @@ -112,21 +112,6 @@ func rgbPickerStyle() renderer.Style { | |||
| 112 | 112 | return style | |
| 113 | 113 | } | |
| 114 | 114 | ||
| 115 | - | func TestPickerRendererResetProtectsRepaintedSessionFromPreviousLease(t *testing.T) { | |
| 116 | - | loop := pickerLoopFixture(t) | |
| 117 | - | r := newPickerRenderer(ansirenderer.ColorProfileTrueColor) | |
| 118 | - | screen := renderer.NewScreen(100, 30) | |
| 119 | - | paintScreen(screen, domain.Size{Cols: 100, Rows: 30}, 'S') | |
| 120 | - | screen.Write(r.render(loop, domain.Size{Cols: 100, Rows: 30}, emptyPickerPreview())) | |
| 121 | - | ||
| 122 | - | r.reset() | |
| 123 | - | paintScreen(screen, domain.Size{Cols: 100, Rows: 30}, 'S') | |
| 124 | - | screen.Write(r.render(loop, domain.Size{Cols: 80, Rows: 30}, emptyPickerPreview())) | |
| 125 | - | ||
| 126 | - | snapshot := screen.Snapshot() | |
| 127 | - | require.Equal(t, 'S', snapshot.Row(3)[72].Rune, "new lease must not clear the previous lease's bounds") | |
| 128 | - | } | |
| 129 | - | ||
| 130 | 115 | func paintScreen(screen *renderer.Screen, size domain.Size, fill rune) { | |
| 131 | 116 | for y := 0; y < size.Rows; y++ { | |
| 132 | 117 | screen.Write([]byte(fmt.Sprintf("\x1b[%d;1H%s", y+1, strings.Repeat(string(fill), size.Cols)))) | |
| @@ -112,21 +112,6 @@ func rgbPickerStyle() renderer.Style { | |||||
| 112 | return style | 112 | return style | ||
| 113 | } | 113 | } | ||
| 114 | 114 | ||||
| 115 | - | func TestPickerRendererResetProtectsRepaintedSessionFromPreviousLease(t *testing.T) { | |||
| 116 | - | loop := pickerLoopFixture(t) | |||
| 117 | - | r := newPickerRenderer(ansirenderer.ColorProfileTrueColor) | |||
| 118 | - | screen := renderer.NewScreen(100, 30) | |||
| 119 | - | paintScreen(screen, domain.Size{Cols: 100, Rows: 30}, 'S') | |||
| 120 | - | screen.Write(r.render(loop, domain.Size{Cols: 100, Rows: 30}, emptyPickerPreview())) | |||
| 121 | - | ||||
| 122 | - | r.reset() | |||
| 123 | - | paintScreen(screen, domain.Size{Cols: 100, Rows: 30}, 'S') | |||
| 124 | - | screen.Write(r.render(loop, domain.Size{Cols: 80, Rows: 30}, emptyPickerPreview())) | |||
| 125 | - | ||||
| 126 | - | snapshot := screen.Snapshot() | |||
| 127 | - | require.Equal(t, 'S', snapshot.Row(3)[72].Rune, "new lease must not clear the previous lease's bounds") | |||
| 128 | - | } | |||
| 129 | - | ||||
| 130 | func paintScreen(screen *renderer.Screen, size domain.Size, fill rune) { | 115 | func paintScreen(screen *renderer.Screen, size domain.Size, fill rune) { | ||
| 131 | for y := 0; y < size.Rows; y++ { | 116 | for y := 0; y < size.Rows; y++ { | ||
| 132 | screen.Write([]byte(fmt.Sprintf("\x1b[%d;1H%s", y+1, strings.Repeat(string(fill), size.Cols)))) | 117 | screen.Write([]byte(fmt.Sprintf("\x1b[%d;1H%s", y+1, strings.Repeat(string(fill), size.Cols)))) | ||
M internal/usecase/client/resize_presentation_test.go +3 -3
| @@ -753,7 +753,7 @@ func TestSupervisorResizeAfterDetachReachesOnlyReplacementAttachment(t *testing. | |||
| 753 | 753 | first := registry.stream(t, 0) | |
| 754 | 754 | deliverReadyStream(t, first) | |
| 755 | 755 | awaitAttachedState(t, harness.sup) | |
| 756 | - | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 1}, harness.sup.attachments.authority.currentToken()) | |
| 756 | + | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 1}, authorityToken(&harness.sup.attachments.authority)) | |
| 757 | 757 | ||
| 758 | 758 | firstResize := domain.Geometry{Size: domain.Size{Cols: 101, Rows: 41}} | |
| 759 | 759 | harness.terminal.resize(firstResize) | |
| @@ -764,7 +764,7 @@ func TestSupervisorResizeAfterDetachReachesOnlyReplacementAttachment(t *testing. | |||
| 764 | 764 | // exists. | |
| 765 | 765 | first.deliver(protocol.Detached{}) | |
| 766 | 766 | awaitPickerState(t, harness.sup) | |
| 767 | - | require.True(t, harness.sup.attachments.authority.currentToken().IsZero(), "the retired token no longer owns the foreground") | |
| 767 | + | require.True(t, authorityToken(&harness.sup.attachments.authority).IsZero(), "the retired token no longer owns the foreground") | |
| 768 | 768 | ||
| 769 | 769 | // A resize in the detach/reattach gap: no attachment owns the foreground, so | |
| 770 | 770 | // the retired stream must not receive it, only the picker repaints, and the | |
| @@ -787,7 +787,7 @@ func TestSupervisorResizeAfterDetachReachesOnlyReplacementAttachment(t *testing. | |||
| 787 | 787 | second := registry.stream(t, 1) | |
| 788 | 788 | deliverReadyStream(t, second) | |
| 789 | 789 | awaitAttachedState(t, harness.sup) | |
| 790 | - | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 2}, harness.sup.attachments.authority.currentToken()) | |
| 790 | + | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 2}, authorityToken(&harness.sup.attachments.authority)) | |
| 791 | 791 | inherited := awaitStreamResizes(t, second, 1) | |
| 792 | 792 | require.Equal(t, gapResize.Size, inherited[0].Size, "the replacement inherits the latest geometry") | |
| 793 | 793 | ||
| @@ -753,7 +753,7 @@ func TestSupervisorResizeAfterDetachReachesOnlyReplacementAttachment(t *testing. | |||||
| 753 | first := registry.stream(t, 0) | 753 | first := registry.stream(t, 0) | ||
| 754 | deliverReadyStream(t, first) | 754 | deliverReadyStream(t, first) | ||
| 755 | awaitAttachedState(t, harness.sup) | 755 | awaitAttachedState(t, harness.sup) | ||
| 756 | - | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 1}, harness.sup.attachments.authority.currentToken()) | 756 | + | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 1}, authorityToken(&harness.sup.attachments.authority)) |
| 757 | 757 | ||||
| 758 | firstResize := domain.Geometry{Size: domain.Size{Cols: 101, Rows: 41}} | 758 | firstResize := domain.Geometry{Size: domain.Size{Cols: 101, Rows: 41}} | ||
| 759 | harness.terminal.resize(firstResize) | 759 | harness.terminal.resize(firstResize) | ||
| @@ -764,7 +764,7 @@ func TestSupervisorResizeAfterDetachReachesOnlyReplacementAttachment(t *testing. | |||||
| 764 | // exists. | 764 | // exists. | ||
| 765 | first.deliver(protocol.Detached{}) | 765 | first.deliver(protocol.Detached{}) | ||
| 766 | awaitPickerState(t, harness.sup) | 766 | awaitPickerState(t, harness.sup) | ||
| 767 | - | require.True(t, harness.sup.attachments.authority.currentToken().IsZero(), "the retired token no longer owns the foreground") | 767 | + | require.True(t, authorityToken(&harness.sup.attachments.authority).IsZero(), "the retired token no longer owns the foreground") |
| 768 | 768 | ||||
| 769 | // A resize in the detach/reattach gap: no attachment owns the foreground, so | 769 | // A resize in the detach/reattach gap: no attachment owns the foreground, so | ||
| 770 | // the retired stream must not receive it, only the picker repaints, and the | 770 | // the retired stream must not receive it, only the picker repaints, and the | ||
| @@ -787,7 +787,7 @@ func TestSupervisorResizeAfterDetachReachesOnlyReplacementAttachment(t *testing. | |||||
| 787 | second := registry.stream(t, 1) | 787 | second := registry.stream(t, 1) | ||
| 788 | deliverReadyStream(t, second) | 788 | deliverReadyStream(t, second) | ||
| 789 | awaitAttachedState(t, harness.sup) | 789 | awaitAttachedState(t, harness.sup) | ||
| 790 | - | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 2}, harness.sup.attachments.authority.currentToken()) | 790 | + | require.Equal(t, AttachmentToken{Generation: 1, Attempt: 2}, authorityToken(&harness.sup.attachments.authority)) |
| 791 | inherited := awaitStreamResizes(t, second, 1) | 791 | inherited := awaitStreamResizes(t, second, 1) | ||
| 792 | require.Equal(t, gapResize.Size, inherited[0].Size, "the replacement inherits the latest geometry") | 792 | require.Equal(t, gapResize.Size, inherited[0].Size, "the replacement inherits the latest geometry") | ||
| 793 | 793 | ||||
M internal/usecase/client/session_picker_cutover_test.go +1 -1
| @@ -50,5 +50,5 @@ func TestSessionPickerDetachRevokesForegroundBeforePicker(t *testing.T) { | |||
| 50 | 50 | awaitStreamHello(t, &sync.Mutex{}, &next) | |
| 51 | 51 | deliverReadyStream(t, next) | |
| 52 | 52 | awaitAttachedState(t, harness.sup) | |
| 53 | - | require.NotEqual(t, foreground.Token(), harness.sup.attachments.authority.currentToken()) | |
| 53 | + | require.NotEqual(t, foreground.Token(), authorityToken(&harness.sup.attachments.authority)) | |
| 54 | 54 | } | |
| @@ -50,5 +50,5 @@ func TestSessionPickerDetachRevokesForegroundBeforePicker(t *testing.T) { | |||||
| 50 | awaitStreamHello(t, &sync.Mutex{}, &next) | 50 | awaitStreamHello(t, &sync.Mutex{}, &next) | ||
| 51 | deliverReadyStream(t, next) | 51 | deliverReadyStream(t, next) | ||
| 52 | awaitAttachedState(t, harness.sup) | 52 | awaitAttachedState(t, harness.sup) | ||
| 53 | - | require.NotEqual(t, foreground.Token(), harness.sup.attachments.authority.currentToken()) | 53 | + | require.NotEqual(t, foreground.Token(), authorityToken(&harness.sup.attachments.authority)) |
| 54 | } | 54 | } | ||
M internal/usecase/client/terminal_input_pump.go +0 -29
| @@ -99,16 +99,6 @@ func newTerminalInputPump(in io.Reader) *terminalInputPump { | |||
| 99 | 99 | } | |
| 100 | 100 | } | |
| 101 | 101 | ||
| 102 | - | // claim grants one attach scanner exclusive permission to dequeue terminal | |
| 103 | - | // input. The reader itself remains lifecycle-owned across reconnects. | |
| 104 | - | func (p *terminalInputPump) claim() uint64 { | |
| 105 | - | consumer, ok := p.tryClaim() | |
| 106 | - | if !ok { | |
| 107 | - | panic("terminal input consumer already claimed") | |
| 108 | - | } | |
| 109 | - | return consumer | |
| 110 | - | } | |
| 111 | - | ||
| 112 | 102 | // tryClaim lets competing foreground owners decline admission without panicking. | |
| 113 | 103 | func (p *terminalInputPump) tryClaim() (uint64, bool) { | |
| 114 | 104 | p.mu.Lock() | |
| @@ -367,15 +357,6 @@ func (p *terminalInputPump) start() { | |||
| 367 | 357 | }) | |
| 368 | 358 | } | |
| 369 | 359 | ||
| 370 | - | func (p *terminalInputPump) hasExited() bool { | |
| 371 | - | select { | |
| 372 | - | case <-p.exited: | |
| 373 | - | return true | |
| 374 | - | default: | |
| 375 | - | return false | |
| 376 | - | } | |
| 377 | - | } | |
| 378 | - | ||
| 379 | 360 | // setReadCause records the cause the terminal reader ended with. An orderly EOF | |
| 380 | 361 | // is the absence of a failure and stays zero, so cause reports io.EOF for it. | |
| 381 | 362 | func (p *terminalInputPump) setReadCause(err error) { | |
| @@ -428,16 +409,6 @@ func (p *terminalInputPump) suspend() { | |||
| 428 | 409 | p.signalClaimChanged() | |
| 429 | 410 | } | |
| 430 | 411 | ||
| 431 | - | func (p *terminalInputPump) resume() { | |
| 432 | - | p.mu.Lock() | |
| 433 | - | if !p.closed && !p.active { | |
| 434 | - | p.activation++ | |
| 435 | - | p.active = true | |
| 436 | - | } | |
| 437 | - | p.mu.Unlock() | |
| 438 | - | p.signalState() | |
| 439 | - | } | |
| 440 | - | ||
| 441 | 412 | func (p *terminalInputPump) stop() { | |
| 442 | 413 | p.stopMu.Do(func() { | |
| 443 | 414 | // Mark closure while holding the same mutex that publishes a completed | |
| @@ -99,16 +99,6 @@ func newTerminalInputPump(in io.Reader) *terminalInputPump { | |||||
| 99 | } | 99 | } | ||
| 100 | } | 100 | } | ||
| 101 | 101 | ||||
| 102 | - | // claim grants one attach scanner exclusive permission to dequeue terminal | |||
| 103 | - | // input. The reader itself remains lifecycle-owned across reconnects. | |||
| 104 | - | func (p *terminalInputPump) claim() uint64 { | |||
| 105 | - | consumer, ok := p.tryClaim() | |||
| 106 | - | if !ok { | |||
| 107 | - | panic("terminal input consumer already claimed") | |||
| 108 | - | } | |||
| 109 | - | return consumer | |||
| 110 | - | } | |||
| 111 | - | ||||
| 112 | // tryClaim lets competing foreground owners decline admission without panicking. | 102 | // tryClaim lets competing foreground owners decline admission without panicking. | ||
| 113 | func (p *terminalInputPump) tryClaim() (uint64, bool) { | 103 | func (p *terminalInputPump) tryClaim() (uint64, bool) { | ||
| 114 | p.mu.Lock() | 104 | p.mu.Lock() | ||
| @@ -367,15 +357,6 @@ func (p *terminalInputPump) start() { | |||||
| 367 | }) | 357 | }) | ||
| 368 | } | 358 | } | ||
| 369 | 359 | ||||
| 370 | - | func (p *terminalInputPump) hasExited() bool { | |||
| 371 | - | select { | |||
| 372 | - | case <-p.exited: | |||
| 373 | - | return true | |||
| 374 | - | default: | |||
| 375 | - | return false | |||
| 376 | - | } | |||
| 377 | - | } | |||
| 378 | - | ||||
| 379 | // setReadCause records the cause the terminal reader ended with. An orderly EOF | 360 | // setReadCause records the cause the terminal reader ended with. An orderly EOF | ||
| 380 | // is the absence of a failure and stays zero, so cause reports io.EOF for it. | 361 | // is the absence of a failure and stays zero, so cause reports io.EOF for it. | ||
| 381 | func (p *terminalInputPump) setReadCause(err error) { | 362 | func (p *terminalInputPump) setReadCause(err error) { | ||
| @@ -428,16 +409,6 @@ func (p *terminalInputPump) suspend() { | |||||
| 428 | p.signalClaimChanged() | 409 | p.signalClaimChanged() | ||
| 429 | } | 410 | } | ||
| 430 | 411 | ||||
| 431 | - | func (p *terminalInputPump) resume() { | |||
| 432 | - | p.mu.Lock() | |||
| 433 | - | if !p.closed && !p.active { | |||
| 434 | - | p.activation++ | |||
| 435 | - | p.active = true | |||
| 436 | - | } | |||
| 437 | - | p.mu.Unlock() | |||
| 438 | - | p.signalState() | |||
| 439 | - | } | |||
| 440 | - | ||||
| 441 | func (p *terminalInputPump) stop() { | 412 | func (p *terminalInputPump) stop() { | ||
| 442 | p.stopMu.Do(func() { | 413 | p.stopMu.Do(func() { | ||
| 443 | // Mark closure while holding the same mutex that publishes a completed | 414 | // Mark closure while holding the same mutex that publishes a completed | ||
M internal/usecase/client/ui_test.go +2 -1
| @@ -22,7 +22,8 @@ func newUITestService(t *testing.T) (*UI, *uiterm.Terminal, *attachPaletteClock, | |||
| 22 | 22 | clock := newAttachPaletteClock() | |
| 23 | 23 | u := NewUI(terminal, clock) | |
| 24 | 24 | input := newTerminalInputPump(nil) | |
| 25 | - | consumer := input.claim() | |
| 25 | + | consumer, ok := input.tryClaim() | |
| 26 | + | require.True(t, ok) | |
| 26 | 27 | generation := u.bindForeground(ctx, input, consumer) | |
| 27 | 28 | view := ports.UIContext{AttachmentHandle: u.Handle(), Generation: generation, Status: ports.UIStatusAttached, OutputEpoch: 1, OutputState: 1, ViewPublication: 1, TabID: "tab", FocusedPaneID: "pane"} | |
| 28 | 29 | view.Route.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: "fixture"} | |
| @@ -22,7 +22,8 @@ func newUITestService(t *testing.T) (*UI, *uiterm.Terminal, *attachPaletteClock, | |||||
| 22 | clock := newAttachPaletteClock() | 22 | clock := newAttachPaletteClock() | ||
| 23 | u := NewUI(terminal, clock) | 23 | u := NewUI(terminal, clock) | ||
| 24 | input := newTerminalInputPump(nil) | 24 | input := newTerminalInputPump(nil) | ||
| 25 | - | consumer := input.claim() | 25 | + | consumer, ok := input.tryClaim() |
| 26 | + | require.True(t, ok) | |||
| 26 | generation := u.bindForeground(ctx, input, consumer) | 27 | generation := u.bindForeground(ctx, input, consumer) | ||
| 27 | view := ports.UIContext{AttachmentHandle: u.Handle(), Generation: generation, Status: ports.UIStatusAttached, OutputEpoch: 1, OutputState: 1, ViewPublication: 1, TabID: "tab", FocusedPaneID: "pane"} | 28 | view := ports.UIContext{AttachmentHandle: u.Handle(), Generation: generation, Status: ports.UIStatusAttached, OutputEpoch: 1, OutputState: 1, ViewPublication: 1, TabID: "tab", FocusedPaneID: "pane"} | ||
| 28 | view.Route.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: "fixture"} | 29 | view.Route.Target = protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: "fixture"} | ||