refactor(client): narrow the picker seam and repaint on publications
A review of the composition seam found the facade too wide and unable to keep the display current. No external consumer exists yet, so the shape changed rather than being frozen. - The facade no longer embeds the controller. It forwards exactly the supervisor's contract plus the two render methods, so a composition can no longer reach the catalogue, flush pending input, or clear a commit before the supervisor reads it. - ResolveInitial is now a named member of the supervisor's picker contract instead of an anonymous type assertion. A facade that omitted it would have made no-argument ephemeral creation silently unavailable with no compile error. - The supervisor repaints after every applied publication, not only on a state transition, so the catalogue and bounded notices cannot go stale between transitions. - The type documents when painting is allowed, that the zero value is inert, and that a composition constructs, assigns, and renders but never drives the contract itself. - The external test asserts identity rather than non-nilness, applies a real observation and renders a frame, and covers the inert zero value. Verified by mutation: removing the publication repaint fails the test that covers it. The immediate first-publication repaint is the same call and is not separately covered.
- commit
- 0ede6cb5eb7197c618326393a6cc7ee3e7e0f096
- author
- brice · · 2026-09-19 12:29 UTC
- parent
- 70034fb
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +205 -42
M internal/usecase/client/picker_composition.go +82 -25
| @@ -9,27 +9,26 @@ import ( | |||
| 9 | 9 | ||
| 10 | 10 | // Picker is the composition seam over the client-owned picker. | |
| 11 | 11 | // | |
| 12 | - | // A composition constructs one picker and assigns it to | |
| 13 | - | // SupervisorConfig.Picker. The supervisor then folds every broker publication | |
| 14 | - | // into it and hands it every terminal read from the single input lifetime it | |
| 15 | - | // owns, so the picker never starts a reader of its own. The supervisor also | |
| 16 | - | // owns raw mode, the terminal writer, and the attachment geometry collector, so | |
| 17 | - | // rendering stays with the composition: it calls Render and RenderNotice with | |
| 18 | - | // the geometry it reads at render time. | |
| 12 | + | // A composition must construct one picker with NewPicker, assign it once to | |
| 13 | + | // SupervisorConfig.Picker, and paint the bytes returned by Render and | |
| 14 | + | // RenderNotice. Its zero value is inert so optional composition remains safe, | |
| 15 | + | // but it does not provide a working picker. | |
| 19 | 16 | // | |
| 20 | - | // The picker deliberately does not consume the terminal's resize channel. That | |
| 21 | - | // channel has one consumer for the whole run, the supervisor's attachment | |
| 22 | - | // geometry collector, so a renderer that also selected on it would compete for | |
| 23 | - | // resizes with the attachment that owns the geometry claimant. | |
| 17 | + | // The remaining exported methods are the supervisor contract: ApplySnapshot, | |
| 18 | + | // TakeOp, ResolveKey, ResolveInitial, SetOwnsInput, OpsReady, and | |
| 19 | + | // ConsumeTerminalRead. A composition must never drive those methods itself. | |
| 20 | + | // Doing so can violate the supervisor's single-reader and single-input-owner | |
| 21 | + | // invariants or consume a pending user decision before the supervisor handles | |
| 22 | + | // it. | |
| 24 | 23 | // | |
| 25 | - | // Assigning this value to SupervisorConfig.Picker is what hands the supervisor | |
| 26 | - | // the picker contract, so the value necessarily also carries the supervisor's | |
| 27 | - | // own entry points (publications, ownership changes, committed keys). A | |
| 28 | - | // composition constructs the picker, assigns it once, and renders it; it never | |
| 29 | - | // drives those entry points itself, because doing so would bypass the single | |
| 30 | - | // input owner the supervisor maintains. | |
| 24 | + | // Painting is allowed only while the supervisor reports PresentPicker. A | |
| 25 | + | // composition must not paint picker bytes during an attachment: the attachment | |
| 26 | + | // writes through the same terminal, and nothing serializes those two writers. | |
| 27 | + | // The picker deliberately neither owns a terminal writer nor consumes the | |
| 28 | + | // terminal resize channel; the composition supplies its current geometry when | |
| 29 | + | // rendering. | |
| 31 | 30 | type Picker struct { | |
| 32 | - | *pickerController | |
| 31 | + | controller *pickerController | |
| 33 | 32 | } | |
| 34 | 33 | ||
| 35 | 34 | // Picker is the supervisor's picker seam: the value assigned to | |
| @@ -39,23 +38,81 @@ var _ pickerHost = (*Picker)(nil) | |||
| 39 | 38 | // NewPicker composes one client-owned picker. A nil clock uses the system | |
| 40 | 39 | // clock, and a zero freshness uses the catalogue default. | |
| 41 | 40 | func NewPicker(clock ports.Clock, freshness time.Duration) *Picker { | |
| 42 | - | return &Picker{pickerController: newPickerController(clock, freshness)} | |
| 41 | + | return &Picker{controller: newPickerController(clock, freshness)} | |
| 42 | + | } | |
| 43 | + | ||
| 44 | + | // ApplySnapshot forwards a broker publication to the supervisor-owned picker. | |
| 45 | + | func (p *Picker) ApplySnapshot(snapshot ports.BrokerSnapshot) { | |
| 46 | + | if p == nil || p.controller == nil { | |
| 47 | + | return | |
| 48 | + | } | |
| 49 | + | p.controller.ApplySnapshot(snapshot) | |
| 50 | + | } | |
| 51 | + | ||
| 52 | + | // TakeOp returns and clears the pending picker operation for the supervisor. | |
| 53 | + | func (p *Picker) TakeOp() (pickerOp, string) { | |
| 54 | + | if p == nil || p.controller == nil { | |
| 55 | + | return pickerOp{}, "" | |
| 56 | + | } | |
| 57 | + | return p.controller.TakeOp() | |
| 58 | + | } | |
| 59 | + | ||
| 60 | + | // ResolveKey resolves a committed catalogue key for the supervisor. | |
| 61 | + | func (p *Picker) ResolveKey(key string, base pickerResolveBase) (ports.BrokerOpenStreamRequest, error) { | |
| 62 | + | if p == nil || p.controller == nil { | |
| 63 | + | return (*pickerController)(nil).ResolveKey(key, base) | |
| 64 | + | } | |
| 65 | + | return p.controller.ResolveKey(key, base) | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | // ResolveInitial resolves the supervisor's one-shot initial navigation. | |
| 69 | + | func (p *Picker) ResolveInitial(navigation InitialNavigation, base pickerResolveBase) (ports.BrokerOpenStreamRequest, error) { | |
| 70 | + | if p == nil || p.controller == nil { | |
| 71 | + | return (*pickerController)(nil).ResolveInitial(navigation, base) | |
| 72 | + | } | |
| 73 | + | return p.controller.ResolveInitial(navigation, base) | |
| 74 | + | } | |
| 75 | + | ||
| 76 | + | // SetOwnsInput updates picker input ownership at an attachment boundary. | |
| 77 | + | func (p *Picker) SetOwnsInput(owns bool) { | |
| 78 | + | if p == nil || p.controller == nil { | |
| 79 | + | return | |
| 80 | + | } | |
| 81 | + | p.controller.SetOwnsInput(owns) | |
| 82 | + | } | |
| 83 | + | ||
| 84 | + | // OpsReady returns the supervisor's coalescing operation wake channel. | |
| 85 | + | func (p *Picker) OpsReady() <-chan struct{} { | |
| 86 | + | if p == nil || p.controller == nil { | |
| 87 | + | return nil | |
| 88 | + | } | |
| 89 | + | return p.controller.OpsReady() | |
| 90 | + | } | |
| 91 | + | ||
| 92 | + | // ConsumeTerminalRead lets the supervisor offer one read to the picker. | |
| 93 | + | func (p *Picker) ConsumeTerminalRead(data []byte) bool { | |
| 94 | + | if p == nil || p.controller == nil { | |
| 95 | + | return false | |
| 96 | + | } | |
| 97 | + | return p.controller.ConsumeTerminalRead(data) | |
| 43 | 98 | } | |
| 44 | 99 | ||
| 45 | 100 | // Render returns the current picker frame for size, or nil when no catalogue | |
| 46 | - | // has been applied yet. | |
| 101 | + | // has been applied yet. A composition paints this payload when Render is | |
| 102 | + | // invoked and the reported presentation is PresentPicker. | |
| 47 | 103 | func (p *Picker) Render(size domain.Size) []byte { | |
| 48 | - | if p == nil || p.pickerController == nil { | |
| 104 | + | if p == nil || p.controller == nil { | |
| 49 | 105 | return nil | |
| 50 | 106 | } | |
| 51 | - | return p.pickerController.Render(size) | |
| 107 | + | return p.controller.Render(size) | |
| 52 | 108 | } | |
| 53 | 109 | ||
| 54 | 110 | // RenderNotice returns the newest bounded notice frame for size, or nil when no | |
| 55 | - | // notice is live. A composition renders it over or beside the picker frame. | |
| 111 | + | // notice is live. A composition renders it over or beside the picker frame only | |
| 112 | + | // while the supervisor reports PresentPicker. | |
| 56 | 113 | func (p *Picker) RenderNotice(size domain.Size) []byte { | |
| 57 | - | if p == nil || p.pickerController == nil { | |
| 114 | + | if p == nil || p.controller == nil { | |
| 58 | 115 | return nil | |
| 59 | 116 | } | |
| 60 | - | return p.pickerController.RenderNotice(size) | |
| 117 | + | return p.controller.RenderNotice(size) | |
| 61 | 118 | } | |
| @@ -9,27 +9,26 @@ import ( | |||||
| 9 | 9 | ||||
| 10 | // Picker is the composition seam over the client-owned picker. | 10 | // Picker is the composition seam over the client-owned picker. | ||
| 11 | // | 11 | // | ||
| 12 | - | // A composition constructs one picker and assigns it to | 12 | + | // A composition must construct one picker with NewPicker, assign it once to |
| 13 | - | // SupervisorConfig.Picker. The supervisor then folds every broker publication | 13 | + | // SupervisorConfig.Picker, and paint the bytes returned by Render and |
| 14 | - | // into it and hands it every terminal read from the single input lifetime it | 14 | + | // RenderNotice. Its zero value is inert so optional composition remains safe, |
| 15 | - | // owns, so the picker never starts a reader of its own. The supervisor also | 15 | + | // but it does not provide a working picker. |
| 16 | - | // owns raw mode, the terminal writer, and the attachment geometry collector, so | |||
| 17 | - | // rendering stays with the composition: it calls Render and RenderNotice with | |||
| 18 | - | // the geometry it reads at render time. | |||
| 19 | // | 16 | // | ||
| 20 | - | // The picker deliberately does not consume the terminal's resize channel. That | 17 | + | // The remaining exported methods are the supervisor contract: ApplySnapshot, |
| 21 | - | // channel has one consumer for the whole run, the supervisor's attachment | 18 | + | // TakeOp, ResolveKey, ResolveInitial, SetOwnsInput, OpsReady, and |
| 22 | - | // geometry collector, so a renderer that also selected on it would compete for | 19 | + | // ConsumeTerminalRead. A composition must never drive those methods itself. |
| 23 | - | // resizes with the attachment that owns the geometry claimant. | 20 | + | // Doing so can violate the supervisor's single-reader and single-input-owner |
| 21 | + | // invariants or consume a pending user decision before the supervisor handles | |||
| 22 | + | // it. | |||
| 24 | // | 23 | // | ||
| 25 | - | // Assigning this value to SupervisorConfig.Picker is what hands the supervisor | 24 | + | // Painting is allowed only while the supervisor reports PresentPicker. A |
| 26 | - | // the picker contract, so the value necessarily also carries the supervisor's | 25 | + | // composition must not paint picker bytes during an attachment: the attachment |
| 27 | - | // own entry points (publications, ownership changes, committed keys). A | 26 | + | // writes through the same terminal, and nothing serializes those two writers. |
| 28 | - | // composition constructs the picker, assigns it once, and renders it; it never | 27 | + | // The picker deliberately neither owns a terminal writer nor consumes the |
| 29 | - | // drives those entry points itself, because doing so would bypass the single | 28 | + | // terminal resize channel; the composition supplies its current geometry when |
| 30 | - | // input owner the supervisor maintains. | 29 | + | // rendering. |
| 31 | type Picker struct { | 30 | type Picker struct { | ||
| 32 | - | *pickerController | 31 | + | controller *pickerController |
| 33 | } | 32 | } | ||
| 34 | 33 | ||||
| 35 | // Picker is the supervisor's picker seam: the value assigned to | 34 | // Picker is the supervisor's picker seam: the value assigned to | ||
| @@ -39,23 +38,81 @@ var _ pickerHost = (*Picker)(nil) | |||||
| 39 | // NewPicker composes one client-owned picker. A nil clock uses the system | 38 | // NewPicker composes one client-owned picker. A nil clock uses the system | ||
| 40 | // clock, and a zero freshness uses the catalogue default. | 39 | // clock, and a zero freshness uses the catalogue default. | ||
| 41 | func NewPicker(clock ports.Clock, freshness time.Duration) *Picker { | 40 | func NewPicker(clock ports.Clock, freshness time.Duration) *Picker { | ||
| 42 | - | return &Picker{pickerController: newPickerController(clock, freshness)} | 41 | + | return &Picker{controller: newPickerController(clock, freshness)} |
| 42 | + | } | |||
| 43 | + | ||||
| 44 | + | // ApplySnapshot forwards a broker publication to the supervisor-owned picker. | |||
| 45 | + | func (p *Picker) ApplySnapshot(snapshot ports.BrokerSnapshot) { | |||
| 46 | + | if p == nil || p.controller == nil { | |||
| 47 | + | return | |||
| 48 | + | } | |||
| 49 | + | p.controller.ApplySnapshot(snapshot) | |||
| 50 | + | } | |||
| 51 | + | ||||
| 52 | + | // TakeOp returns and clears the pending picker operation for the supervisor. | |||
| 53 | + | func (p *Picker) TakeOp() (pickerOp, string) { | |||
| 54 | + | if p == nil || p.controller == nil { | |||
| 55 | + | return pickerOp{}, "" | |||
| 56 | + | } | |||
| 57 | + | return p.controller.TakeOp() | |||
| 58 | + | } | |||
| 59 | + | ||||
| 60 | + | // ResolveKey resolves a committed catalogue key for the supervisor. | |||
| 61 | + | func (p *Picker) ResolveKey(key string, base pickerResolveBase) (ports.BrokerOpenStreamRequest, error) { | |||
| 62 | + | if p == nil || p.controller == nil { | |||
| 63 | + | return (*pickerController)(nil).ResolveKey(key, base) | |||
| 64 | + | } | |||
| 65 | + | return p.controller.ResolveKey(key, base) | |||
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | // ResolveInitial resolves the supervisor's one-shot initial navigation. | |||
| 69 | + | func (p *Picker) ResolveInitial(navigation InitialNavigation, base pickerResolveBase) (ports.BrokerOpenStreamRequest, error) { | |||
| 70 | + | if p == nil || p.controller == nil { | |||
| 71 | + | return (*pickerController)(nil).ResolveInitial(navigation, base) | |||
| 72 | + | } | |||
| 73 | + | return p.controller.ResolveInitial(navigation, base) | |||
| 74 | + | } | |||
| 75 | + | ||||
| 76 | + | // SetOwnsInput updates picker input ownership at an attachment boundary. | |||
| 77 | + | func (p *Picker) SetOwnsInput(owns bool) { | |||
| 78 | + | if p == nil || p.controller == nil { | |||
| 79 | + | return | |||
| 80 | + | } | |||
| 81 | + | p.controller.SetOwnsInput(owns) | |||
| 82 | + | } | |||
| 83 | + | ||||
| 84 | + | // OpsReady returns the supervisor's coalescing operation wake channel. | |||
| 85 | + | func (p *Picker) OpsReady() <-chan struct{} { | |||
| 86 | + | if p == nil || p.controller == nil { | |||
| 87 | + | return nil | |||
| 88 | + | } | |||
| 89 | + | return p.controller.OpsReady() | |||
| 90 | + | } | |||
| 91 | + | ||||
| 92 | + | // ConsumeTerminalRead lets the supervisor offer one read to the picker. | |||
| 93 | + | func (p *Picker) ConsumeTerminalRead(data []byte) bool { | |||
| 94 | + | if p == nil || p.controller == nil { | |||
| 95 | + | return false | |||
| 96 | + | } | |||
| 97 | + | return p.controller.ConsumeTerminalRead(data) | |||
| 43 | } | 98 | } | ||
| 44 | 99 | ||||
| 45 | // Render returns the current picker frame for size, or nil when no catalogue | 100 | // Render returns the current picker frame for size, or nil when no catalogue | ||
| 46 | - | // has been applied yet. | 101 | + | // has been applied yet. A composition paints this payload when Render is |
| 102 | + | // invoked and the reported presentation is PresentPicker. | |||
| 47 | func (p *Picker) Render(size domain.Size) []byte { | 103 | func (p *Picker) Render(size domain.Size) []byte { | ||
| 48 | - | if p == nil || p.pickerController == nil { | 104 | + | if p == nil || p.controller == nil { |
| 49 | return nil | 105 | return nil | ||
| 50 | } | 106 | } | ||
| 51 | - | return p.pickerController.Render(size) | 107 | + | return p.controller.Render(size) |
| 52 | } | 108 | } | ||
| 53 | 109 | ||||
| 54 | // RenderNotice returns the newest bounded notice frame for size, or nil when no | 110 | // RenderNotice returns the newest bounded notice frame for size, or nil when no | ||
| 55 | - | // notice is live. A composition renders it over or beside the picker frame. | 111 | + | // notice is live. A composition renders it over or beside the picker frame only |
| 112 | + | // while the supervisor reports PresentPicker. | |||
| 56 | func (p *Picker) RenderNotice(size domain.Size) []byte { | 113 | func (p *Picker) RenderNotice(size domain.Size) []byte { | ||
| 57 | - | if p == nil || p.pickerController == nil { | 114 | + | if p == nil || p.controller == nil { |
| 58 | return nil | 115 | return nil | ||
| 59 | } | 116 | } | ||
| 60 | - | return p.pickerController.RenderNotice(size) | 117 | + | return p.controller.RenderNotice(size) |
| 61 | } | 118 | } | ||
M internal/usecase/client/picker_composition_external_test.go +53 -7
| @@ -2,10 +2,14 @@ package client_test | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "testing" | |
| 5 | + | "time" | |
| 5 | 6 | ||
| 6 | 7 | "github.com/stretchr/testify/require" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/vev/internal/domain" | |
| 10 | + | "github.com/bnema/vev/internal/ports" | |
| 11 | + | "github.com/bnema/vev/internal/protocol" | |
| 12 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |
| 9 | 13 | "github.com/bnema/vev/internal/usecase/client" | |
| 10 | 14 | ) | |
| 11 | 15 | ||
| @@ -20,17 +24,59 @@ func TestCompositionOutsideThePackageCanBuildThePicker(t *testing.T) { | |||
| 20 | 24 | ||
| 21 | 25 | // The exact assignment an app composition performs. | |
| 22 | 26 | cfg := client.SupervisorConfig{Picker: picker} | |
| 23 | - | require.NotNil(t, cfg.Picker) | |
| 27 | + | require.Same(t, picker, cfg.Picker) | |
| 24 | 28 | ||
| 25 | - | // Rendering belongs to the composition, so both entry points are callable | |
| 26 | - | // with the geometry the composition reads itself. Nothing has been applied | |
| 27 | - | // yet, so both are empty rather than a frame. | |
| 28 | - | require.Empty(t, picker.Render(geometry)) | |
| 29 | + | // Exercise a real local-only publication through the exported facade. | |
| 30 | + | picker.ApplySnapshot(ports.BrokerSnapshot{ | |
| 31 | + | Epoch: 1, | |
| 32 | + | Revision: 1, | |
| 33 | + | Daemons: []ports.BrokerDaemonObservation{{ | |
| 34 | + | Local: true, | |
| 35 | + | DisplayOrigin: "local", | |
| 36 | + | Policy: validExternalPickerPolicy(), | |
| 37 | + | Identity: "local-daemon", | |
| 38 | + | Incarnation: ports.BrokerDaemonIncarnation{1}, | |
| 39 | + | ProtocolVersion: protocol.Version, | |
| 40 | + | Availability: domain.RemoteAvailabilityReachable, | |
| 41 | + | LastSuccess: time.Now(), | |
| 42 | + | InventoryKnown: true, | |
| 43 | + | Sessions: []catalogue.RemoteCatalogSession{{ | |
| 44 | + | LifecycleID: domain.SessionLifecycleID{1}, | |
| 45 | + | Name: "work", | |
| 46 | + | State: catalogue.RemoteCatalogSessionUp, | |
| 47 | + | }}, | |
| 48 | + | }}, | |
| 49 | + | }) | |
| 50 | + | require.NotEmpty(t, picker.Render(geometry)) | |
| 29 | 51 | require.Empty(t, picker.RenderNotice(geometry)) | |
| 52 | + | } | |
| 53 | + | ||
| 54 | + | func TestPickerZeroValueIsInertOutsideThePackage(t *testing.T) { | |
| 55 | + | geometry := domain.Size{Cols: 80, Rows: 24} | |
| 56 | + | zero := &client.Picker{} | |
| 30 | 57 | ||
| 31 | - | // A composition that never built a picker renders nothing instead of | |
| 32 | - | // panicking. | |
| 58 | + | require.NotPanics(t, func() { | |
| 59 | + | zero.ApplySnapshot(ports.BrokerSnapshot{Epoch: 1, Revision: 1}) | |
| 60 | + | zero.SetOwnsInput(true) | |
| 61 | + | }) | |
| 62 | + | require.Nil(t, zero.OpsReady()) | |
| 63 | + | require.Empty(t, zero.Render(geometry)) | |
| 64 | + | require.Empty(t, zero.RenderNotice(geometry)) | |
| 65 | + | ||
| 66 | + | // An absent optional picker is inert too. | |
| 33 | 67 | var absent *client.Picker | |
| 34 | 68 | require.Empty(t, absent.Render(geometry)) | |
| 35 | 69 | require.Empty(t, absent.RenderNotice(geometry)) | |
| 36 | 70 | } | |
| 71 | + | ||
| 72 | + | func validExternalPickerPolicy() ports.BrokerPolicy { | |
| 73 | + | return ports.BrokerPolicy{ | |
| 74 | + | ProtocolVersion: protocol.Version, | |
| 75 | + | CatalogSchemaVersion: catalogue.RemoteCatalogSchemaVersion, | |
| 76 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, | |
| 77 | + | Transport: "quic", | |
| 78 | + | Trust: "trust", | |
| 79 | + | Launch: "launch", | |
| 80 | + | Isolation: "user", | |
| 81 | + | } | |
| 82 | + | } | |
| @@ -2,10 +2,14 @@ package client_test | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "testing" | 4 | "testing" | ||
| 5 | + | "time" | |||
| 5 | 6 | ||||
| 6 | "github.com/stretchr/testify/require" | 7 | "github.com/stretchr/testify/require" | ||
| 7 | 8 | ||||
| 8 | "github.com/bnema/vev/internal/domain" | 9 | "github.com/bnema/vev/internal/domain" | ||
| 10 | + | "github.com/bnema/vev/internal/ports" | |||
| 11 | + | "github.com/bnema/vev/internal/protocol" | |||
| 12 | + | "github.com/bnema/vev/internal/protocol/catalogue" | |||
| 9 | "github.com/bnema/vev/internal/usecase/client" | 13 | "github.com/bnema/vev/internal/usecase/client" | ||
| 10 | ) | 14 | ) | ||
| 11 | 15 | ||||
| @@ -20,17 +24,59 @@ func TestCompositionOutsideThePackageCanBuildThePicker(t *testing.T) { | |||||
| 20 | 24 | ||||
| 21 | // The exact assignment an app composition performs. | 25 | // The exact assignment an app composition performs. | ||
| 22 | cfg := client.SupervisorConfig{Picker: picker} | 26 | cfg := client.SupervisorConfig{Picker: picker} | ||
| 23 | - | require.NotNil(t, cfg.Picker) | 27 | + | require.Same(t, picker, cfg.Picker) |
| 24 | 28 | ||||
| 25 | - | // Rendering belongs to the composition, so both entry points are callable | 29 | + | // Exercise a real local-only publication through the exported facade. |
| 26 | - | // with the geometry the composition reads itself. Nothing has been applied | 30 | + | picker.ApplySnapshot(ports.BrokerSnapshot{ |
| 27 | - | // yet, so both are empty rather than a frame. | 31 | + | Epoch: 1, |
| 28 | - | require.Empty(t, picker.Render(geometry)) | 32 | + | Revision: 1, |
| 33 | + | Daemons: []ports.BrokerDaemonObservation{{ | |||
| 34 | + | Local: true, | |||
| 35 | + | DisplayOrigin: "local", | |||
| 36 | + | Policy: validExternalPickerPolicy(), | |||
| 37 | + | Identity: "local-daemon", | |||
| 38 | + | Incarnation: ports.BrokerDaemonIncarnation{1}, | |||
| 39 | + | ProtocolVersion: protocol.Version, | |||
| 40 | + | Availability: domain.RemoteAvailabilityReachable, | |||
| 41 | + | LastSuccess: time.Now(), | |||
| 42 | + | InventoryKnown: true, | |||
| 43 | + | Sessions: []catalogue.RemoteCatalogSession{{ | |||
| 44 | + | LifecycleID: domain.SessionLifecycleID{1}, | |||
| 45 | + | Name: "work", | |||
| 46 | + | State: catalogue.RemoteCatalogSessionUp, | |||
| 47 | + | }}, | |||
| 48 | + | }}, | |||
| 49 | + | }) | |||
| 50 | + | require.NotEmpty(t, picker.Render(geometry)) | |||
| 29 | require.Empty(t, picker.RenderNotice(geometry)) | 51 | require.Empty(t, picker.RenderNotice(geometry)) | ||
| 52 | + | } | |||
| 53 | + | ||||
| 54 | + | func TestPickerZeroValueIsInertOutsideThePackage(t *testing.T) { | |||
| 55 | + | geometry := domain.Size{Cols: 80, Rows: 24} | |||
| 56 | + | zero := &client.Picker{} | |||
| 30 | 57 | ||||
| 31 | - | // A composition that never built a picker renders nothing instead of | 58 | + | require.NotPanics(t, func() { |
| 32 | - | // panicking. | 59 | + | zero.ApplySnapshot(ports.BrokerSnapshot{Epoch: 1, Revision: 1}) |
| 60 | + | zero.SetOwnsInput(true) | |||
| 61 | + | }) | |||
| 62 | + | require.Nil(t, zero.OpsReady()) | |||
| 63 | + | require.Empty(t, zero.Render(geometry)) | |||
| 64 | + | require.Empty(t, zero.RenderNotice(geometry)) | |||
| 65 | + | ||||
| 66 | + | // An absent optional picker is inert too. | |||
| 33 | var absent *client.Picker | 67 | var absent *client.Picker | ||
| 34 | require.Empty(t, absent.Render(geometry)) | 68 | require.Empty(t, absent.Render(geometry)) | ||
| 35 | require.Empty(t, absent.RenderNotice(geometry)) | 69 | require.Empty(t, absent.RenderNotice(geometry)) | ||
| 36 | } | 70 | } | ||
| 71 | + | ||||
| 72 | + | func validExternalPickerPolicy() ports.BrokerPolicy { | |||
| 73 | + | return ports.BrokerPolicy{ | |||
| 74 | + | ProtocolVersion: protocol.Version, | |||
| 75 | + | CatalogSchemaVersion: catalogue.RemoteCatalogSchemaVersion, | |||
| 76 | + | EnvironmentPolicy: protocol.EnvironmentPolicyDaemonOwned, | |||
| 77 | + | Transport: "quic", | |||
| 78 | + | Trust: "trust", | |||
| 79 | + | Launch: "launch", | |||
| 80 | + | Isolation: "user", | |||
| 81 | + | } | |||
| 82 | + | } | |||
M internal/usecase/client/picker_controller.go +3 -0
| @@ -55,6 +55,9 @@ type pickerHost interface { | |||
| 55 | 55 | // ResolveKey revalidates exactly one committed catalogue key into the | |
| 56 | 56 | // exact broker stream request the user committed. | |
| 57 | 57 | ResolveKey(string, pickerResolveBase) (ports.BrokerOpenStreamRequest, error) | |
| 58 | + | // ResolveInitial resolves the one-shot initial navigation without relying | |
| 59 | + | // on mutable presentation selection. | |
| 60 | + | ResolveInitial(InitialNavigation, pickerResolveBase) (ports.BrokerOpenStreamRequest, error) | |
| 58 | 61 | // SetOwnsInput releases or re-acquires picker input ownership at an attach | |
| 59 | 62 | // boundary, so exactly one owner consumes the shared terminal reader. | |
| 60 | 63 | SetOwnsInput(bool) | |
| @@ -55,6 +55,9 @@ type pickerHost interface { | |||||
| 55 | // ResolveKey revalidates exactly one committed catalogue key into the | 55 | // ResolveKey revalidates exactly one committed catalogue key into the | ||
| 56 | // exact broker stream request the user committed. | 56 | // exact broker stream request the user committed. | ||
| 57 | ResolveKey(string, pickerResolveBase) (ports.BrokerOpenStreamRequest, error) | 57 | ResolveKey(string, pickerResolveBase) (ports.BrokerOpenStreamRequest, error) | ||
| 58 | + | // ResolveInitial resolves the one-shot initial navigation without relying | |||
| 59 | + | // on mutable presentation selection. | |||
| 60 | + | ResolveInitial(InitialNavigation, pickerResolveBase) (ports.BrokerOpenStreamRequest, error) | |||
| 58 | // SetOwnsInput releases or re-acquires picker input ownership at an attach | 61 | // SetOwnsInput releases or re-acquires picker input ownership at an attach | ||
| 59 | // boundary, so exactly one owner consumes the shared terminal reader. | 62 | // boundary, so exactly one owner consumes the shared terminal reader. | ||
| 60 | SetOwnsInput(bool) | 63 | SetOwnsInput(bool) | ||
M internal/usecase/client/supervisor.go +18 -3
| @@ -260,8 +260,10 @@ type SupervisorConfig struct { | |||
| 260 | 260 | // Clock supplies the retry timers. Required; use ports.Clock, not the wall | |
| 261 | 261 | // clock. | |
| 262 | 262 | Clock ports.Clock | |
| 263 | - | // Render paints the current state. It is called once per visible | |
| 264 | - | // transition. Optional; the default renders nothing. | |
| 263 | + | // Render asks the composition to paint the current state. It is called for | |
| 264 | + | // visible transitions and after picker catalogue publications; while the | |
| 265 | + | // state reports PresentPicker, the composition paints from Picker.Render | |
| 266 | + | // and Picker.RenderNotice. Optional; the default renders nothing. | |
| 265 | 267 | Render func(State) | |
| 266 | 268 | // Notify surfaces a connectivity failure without leaving the picker. | |
| 267 | 269 | // Optional; the default notifies nothing. | |
| @@ -502,6 +504,7 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) { | |||
| 502 | 504 | // for the next one; the picker never shows a stale empty catalogue | |
| 503 | 505 | // while an established connection already has state. | |
| 504 | 506 | s.cfg.Picker.ApplySnapshot(service.Snapshot()) | |
| 507 | + | s.renderCurrent() | |
| 505 | 508 | } | |
| 506 | 509 | wasReadyLost := s.State().ReadyLost | |
| 507 | 510 | s.transition(supervisorEvent{kind: supervisorReady}) | |
| @@ -727,6 +730,7 @@ func (s *Supervisor) awaitReady(ctx context.Context, input *terminalInputLifetim | |||
| 727 | 730 | return readyOutcome{lossErr: service.Err()} | |
| 728 | 731 | case <-changed: | |
| 729 | 732 | s.cfg.Picker.ApplySnapshot(service.Snapshot()) | |
| 733 | + | s.renderCurrent() | |
| 730 | 734 | case <-ops: | |
| 731 | 735 | if key, ok := s.takeCommittedKey(); ok { | |
| 732 | 736 | return readyOutcome{commitKey: key} | |
| @@ -904,7 +908,18 @@ func (s *Supervisor) transition(event supervisorEvent) { | |||
| 904 | 908 | s.state = reduceSupervisor(s.state, event) | |
| 905 | 909 | state := s.state | |
| 906 | 910 | s.mu.Unlock() | |
| 907 | - | s.cfg.Render(state) | |
| 911 | + | if s.cfg.Render != nil { | |
| 912 | + | s.cfg.Render(state) | |
| 913 | + | } | |
| 914 | + | } | |
| 915 | + | ||
| 916 | + | // renderCurrent repaints presentation data that changed without a supervisor | |
| 917 | + | // state transition, notably a picker catalogue publication. | |
| 918 | + | func (s *Supervisor) renderCurrent() { | |
| 919 | + | if s == nil || s.cfg.Render == nil { | |
| 920 | + | return | |
| 921 | + | } | |
| 922 | + | s.cfg.Render(s.State()) | |
| 908 | 923 | } | |
| 909 | 924 | ||
| 910 | 925 | // terminalInputLifetime owns the single read of the controlling terminal. It is | |
| @@ -260,8 +260,10 @@ type SupervisorConfig struct { | |||||
| 260 | // Clock supplies the retry timers. Required; use ports.Clock, not the wall | 260 | // Clock supplies the retry timers. Required; use ports.Clock, not the wall | ||
| 261 | // clock. | 261 | // clock. | ||
| 262 | Clock ports.Clock | 262 | Clock ports.Clock | ||
| 263 | - | // Render paints the current state. It is called once per visible | 263 | + | // Render asks the composition to paint the current state. It is called for |
| 264 | - | // transition. Optional; the default renders nothing. | 264 | + | // visible transitions and after picker catalogue publications; while the |
| 265 | + | // state reports PresentPicker, the composition paints from Picker.Render | |||
| 266 | + | // and Picker.RenderNotice. Optional; the default renders nothing. | |||
| 265 | Render func(State) | 267 | Render func(State) | ||
| 266 | // Notify surfaces a connectivity failure without leaving the picker. | 268 | // Notify surfaces a connectivity failure without leaving the picker. | ||
| 267 | // Optional; the default notifies nothing. | 269 | // Optional; the default notifies nothing. | ||
| @@ -502,6 +504,7 @@ func (s *Supervisor) Run(ctx context.Context) (retErr error) { | |||||
| 502 | // for the next one; the picker never shows a stale empty catalogue | 504 | // for the next one; the picker never shows a stale empty catalogue | ||
| 503 | // while an established connection already has state. | 505 | // while an established connection already has state. | ||
| 504 | s.cfg.Picker.ApplySnapshot(service.Snapshot()) | 506 | s.cfg.Picker.ApplySnapshot(service.Snapshot()) | ||
| 507 | + | s.renderCurrent() | |||
| 505 | } | 508 | } | ||
| 506 | wasReadyLost := s.State().ReadyLost | 509 | wasReadyLost := s.State().ReadyLost | ||
| 507 | s.transition(supervisorEvent{kind: supervisorReady}) | 510 | s.transition(supervisorEvent{kind: supervisorReady}) | ||
| @@ -727,6 +730,7 @@ func (s *Supervisor) awaitReady(ctx context.Context, input *terminalInputLifetim | |||||
| 727 | return readyOutcome{lossErr: service.Err()} | 730 | return readyOutcome{lossErr: service.Err()} | ||
| 728 | case <-changed: | 731 | case <-changed: | ||
| 729 | s.cfg.Picker.ApplySnapshot(service.Snapshot()) | 732 | s.cfg.Picker.ApplySnapshot(service.Snapshot()) | ||
| 733 | + | s.renderCurrent() | |||
| 730 | case <-ops: | 734 | case <-ops: | ||
| 731 | if key, ok := s.takeCommittedKey(); ok { | 735 | if key, ok := s.takeCommittedKey(); ok { | ||
| 732 | return readyOutcome{commitKey: key} | 736 | return readyOutcome{commitKey: key} | ||
| @@ -904,7 +908,18 @@ func (s *Supervisor) transition(event supervisorEvent) { | |||||
| 904 | s.state = reduceSupervisor(s.state, event) | 908 | s.state = reduceSupervisor(s.state, event) | ||
| 905 | state := s.state | 909 | state := s.state | ||
| 906 | s.mu.Unlock() | 910 | s.mu.Unlock() | ||
| 907 | - | s.cfg.Render(state) | 911 | + | if s.cfg.Render != nil { |
| 912 | + | s.cfg.Render(state) | |||
| 913 | + | } | |||
| 914 | + | } | |||
| 915 | + | ||||
| 916 | + | // renderCurrent repaints presentation data that changed without a supervisor | |||
| 917 | + | // state transition, notably a picker catalogue publication. | |||
| 918 | + | func (s *Supervisor) renderCurrent() { | |||
| 919 | + | if s == nil || s.cfg.Render == nil { | |||
| 920 | + | return | |||
| 921 | + | } | |||
| 922 | + | s.cfg.Render(s.State()) | |||
| 908 | } | 923 | } | ||
| 909 | 924 | ||||
| 910 | // terminalInputLifetime owns the single read of the controlling terminal. It is | 925 | // terminalInputLifetime owns the single read of the controlling terminal. It is | ||
M internal/usecase/client/supervisor_attach.go +1 -7
| @@ -239,13 +239,7 @@ type attachmentSettlement struct { | |||
| 239 | 239 | // allocated strictly increasing per connection and is consumed even when | |
| 240 | 240 | // resolution refuses, matching the pool's never-reused ID contract. | |
| 241 | 241 | func (s *Supervisor) resolveInitialStreamRequest(service ports.BrokerService, navigation InitialNavigation) (ports.BrokerOpenStreamRequest, error) { | |
| 242 | - | resolver, ok := s.cfg.Picker.(interface { | |
| 243 | - | ResolveInitial(InitialNavigation, pickerResolveBase) (ports.BrokerOpenStreamRequest, error) | |
| 244 | - | }) | |
| 245 | - | if !ok { | |
| 246 | - | return ports.BrokerOpenStreamRequest{}, pickerCatalogueError{Code: pickerCatalogueNoSelection, Text: "initial navigation is unavailable"} | |
| 247 | - | } | |
| 248 | - | return resolver.ResolveInitial(navigation, pickerResolveBase{Connection: service.ConnectionID(), Stream: ports.BrokerStreamID(s.nextStream.Add(1))}) | |
| 242 | + | return s.cfg.Picker.ResolveInitial(navigation, pickerResolveBase{Connection: service.ConnectionID(), Stream: ports.BrokerStreamID(s.nextStream.Add(1))}) | |
| 249 | 243 | } | |
| 250 | 244 | ||
| 251 | 245 | func (s *Supervisor) resolveCommittedStreamRequest(service ports.BrokerService, key string) (ports.BrokerOpenStreamRequest, error) { | |
| @@ -239,13 +239,7 @@ type attachmentSettlement struct { | |||||
| 239 | // allocated strictly increasing per connection and is consumed even when | 239 | // allocated strictly increasing per connection and is consumed even when | ||
| 240 | // resolution refuses, matching the pool's never-reused ID contract. | 240 | // resolution refuses, matching the pool's never-reused ID contract. | ||
| 241 | func (s *Supervisor) resolveInitialStreamRequest(service ports.BrokerService, navigation InitialNavigation) (ports.BrokerOpenStreamRequest, error) { | 241 | func (s *Supervisor) resolveInitialStreamRequest(service ports.BrokerService, navigation InitialNavigation) (ports.BrokerOpenStreamRequest, error) { | ||
| 242 | - | resolver, ok := s.cfg.Picker.(interface { | 242 | + | return s.cfg.Picker.ResolveInitial(navigation, pickerResolveBase{Connection: service.ConnectionID(), Stream: ports.BrokerStreamID(s.nextStream.Add(1))}) |
| 243 | - | ResolveInitial(InitialNavigation, pickerResolveBase) (ports.BrokerOpenStreamRequest, error) | |||
| 244 | - | }) | |||
| 245 | - | if !ok { | |||
| 246 | - | return ports.BrokerOpenStreamRequest{}, pickerCatalogueError{Code: pickerCatalogueNoSelection, Text: "initial navigation is unavailable"} | |||
| 247 | - | } | |||
| 248 | - | return resolver.ResolveInitial(navigation, pickerResolveBase{Connection: service.ConnectionID(), Stream: ports.BrokerStreamID(s.nextStream.Add(1))}) | |||
| 249 | } | 243 | } | ||
| 250 | 244 | ||||
| 251 | func (s *Supervisor) resolveCommittedStreamRequest(service ports.BrokerService, key string) (ports.BrokerOpenStreamRequest, error) { | 245 | func (s *Supervisor) resolveCommittedStreamRequest(service ports.BrokerService, key string) (ports.BrokerOpenStreamRequest, error) { | ||
M internal/usecase/client/supervisor_test.go +48 -0
| @@ -647,6 +647,54 @@ func TestSupervisorPickerWithoutBroker(t *testing.T) { | |||
| 647 | 647 | } | |
| 648 | 648 | } | |
| 649 | 649 | ||
| 650 | + | func TestSupervisorPickerPublicationRequestsRepaint(t *testing.T) { | |
| 651 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 652 | + | defer cancel() | |
| 653 | + | ||
| 654 | + | clock := newSupervisorTestClock() | |
| 655 | + | reader := newSupervisorTestReader() | |
| 656 | + | t.Cleanup(reader.unblock) | |
| 657 | + | service := newSupervisorTestService(ports.BrokerConnectionID{1}) | |
| 658 | + | service.publish(1, 1) | |
| 659 | + | renders := make(chan State, 16) | |
| 660 | + | ||
| 661 | + | sup := mustSupervisor(t, SupervisorConfig{ | |
| 662 | + | Connector: newSupervisorTestConnector(func(context.Context, int) (ports.BrokerService, error) { | |
| 663 | + | return service, nil | |
| 664 | + | }), | |
| 665 | + | Terminal: newSupervisorTestTerminal(reader), | |
| 666 | + | Clock: clock, | |
| 667 | + | Picker: newPickerController(clock, pickerTestFreshness), | |
| 668 | + | Render: func(state State) { | |
| 669 | + | renders <- state | |
| 670 | + | }, | |
| 671 | + | }) | |
| 672 | + | ||
| 673 | + | runErr := make(chan error, 1) | |
| 674 | + | go func() { runErr <- sup.Run(ctx) }() | |
| 675 | + | require.Eventually(t, func() bool { return sup.State().Connectivity == ConnectivityReady }, 5*time.Second, time.Millisecond) | |
| 676 | + | for { | |
| 677 | + | select { | |
| 678 | + | case <-renders: | |
| 679 | + | default: | |
| 680 | + | goto drained | |
| 681 | + | } | |
| 682 | + | } | |
| 683 | + | ||
| 684 | + | drained: | |
| 685 | + | service.publish(1, 2) | |
| 686 | + | select { | |
| 687 | + | case state := <-renders: | |
| 688 | + | require.Equal(t, PresentPicker, state.Presentation) | |
| 689 | + | require.Equal(t, ConnectivityReady, state.Connectivity) | |
| 690 | + | case <-time.After(5 * time.Second): | |
| 691 | + | t.Fatal("picker publication did not request a repaint") | |
| 692 | + | } | |
| 693 | + | ||
| 694 | + | cancel() | |
| 695 | + | require.ErrorIs(t, <-runErr, context.Canceled) | |
| 696 | + | } | |
| 697 | + | ||
| 650 | 698 | func TestSupervisorBrokerLossReconnectsWithFreshGeneration(t *testing.T) { | |
| 651 | 699 | ctx, cancel := context.WithCancel(context.Background()) | |
| 652 | 700 | defer cancel() | |
| @@ -647,6 +647,54 @@ func TestSupervisorPickerWithoutBroker(t *testing.T) { | |||||
| 647 | } | 647 | } | ||
| 648 | } | 648 | } | ||
| 649 | 649 | ||||
| 650 | + | func TestSupervisorPickerPublicationRequestsRepaint(t *testing.T) { | |||
| 651 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 652 | + | defer cancel() | |||
| 653 | + | ||||
| 654 | + | clock := newSupervisorTestClock() | |||
| 655 | + | reader := newSupervisorTestReader() | |||
| 656 | + | t.Cleanup(reader.unblock) | |||
| 657 | + | service := newSupervisorTestService(ports.BrokerConnectionID{1}) | |||
| 658 | + | service.publish(1, 1) | |||
| 659 | + | renders := make(chan State, 16) | |||
| 660 | + | ||||
| 661 | + | sup := mustSupervisor(t, SupervisorConfig{ | |||
| 662 | + | Connector: newSupervisorTestConnector(func(context.Context, int) (ports.BrokerService, error) { | |||
| 663 | + | return service, nil | |||
| 664 | + | }), | |||
| 665 | + | Terminal: newSupervisorTestTerminal(reader), | |||
| 666 | + | Clock: clock, | |||
| 667 | + | Picker: newPickerController(clock, pickerTestFreshness), | |||
| 668 | + | Render: func(state State) { | |||
| 669 | + | renders <- state | |||
| 670 | + | }, | |||
| 671 | + | }) | |||
| 672 | + | ||||
| 673 | + | runErr := make(chan error, 1) | |||
| 674 | + | go func() { runErr <- sup.Run(ctx) }() | |||
| 675 | + | require.Eventually(t, func() bool { return sup.State().Connectivity == ConnectivityReady }, 5*time.Second, time.Millisecond) | |||
| 676 | + | for { | |||
| 677 | + | select { | |||
| 678 | + | case <-renders: | |||
| 679 | + | default: | |||
| 680 | + | goto drained | |||
| 681 | + | } | |||
| 682 | + | } | |||
| 683 | + | ||||
| 684 | + | drained: | |||
| 685 | + | service.publish(1, 2) | |||
| 686 | + | select { | |||
| 687 | + | case state := <-renders: | |||
| 688 | + | require.Equal(t, PresentPicker, state.Presentation) | |||
| 689 | + | require.Equal(t, ConnectivityReady, state.Connectivity) | |||
| 690 | + | case <-time.After(5 * time.Second): | |||
| 691 | + | t.Fatal("picker publication did not request a repaint") | |||
| 692 | + | } | |||
| 693 | + | ||||
| 694 | + | cancel() | |||
| 695 | + | require.ErrorIs(t, <-runErr, context.Canceled) | |||
| 696 | + | } | |||
| 697 | + | ||||
| 650 | func TestSupervisorBrokerLossReconnectsWithFreshGeneration(t *testing.T) { | 698 | func TestSupervisorBrokerLossReconnectsWithFreshGeneration(t *testing.T) { | ||
| 651 | ctx, cancel := context.WithCancel(context.Background()) | 699 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 652 | defer cancel() | 700 | defer cancel() | ||