fix(daemon): publish proxied metadata on empty frames
emitFrame skipped sendSessionMetaIfChanged whenever the terminal frame produced no bytes, so a proxied client kept a stale SessionMeta until the next visible repaint. Send metadata independently of the output bytes while preserving the metadata-before-output ordering. Also fold the shared transport-incarnation validation and ticket admission into beginExpectedTransportSendLocked, return a package-level errSessionMetaUnavailable sentinel, add payloadWriter.putBool for the repeated boolean writes (encoding unchanged), and tighten the proxied contract tests.
- commit
- af38802d922a6df9ddb0f5f99a2fe7b1f925823f
- author
- brice · · 2026-07-31 09:22 UTC
- parent
- efdd7d6
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +153 -89
M internal/ports/wire.go +13 -15
| @@ -254,6 +254,16 @@ func (w *payloadWriter) putUint8(v uint8) { | |||
| 254 | 254 | w.b = append(w.b, v) | |
| 255 | 255 | } | |
| 256 | 256 | ||
| 257 | + | // putBool writes the single byte payloadReader.getBool accepts: 1 for true and | |
| 258 | + | // 0 for false. | |
| 259 | + | func (w *payloadWriter) putBool(v bool) { | |
| 260 | + | if v { | |
| 261 | + | w.putUint8(1) | |
| 262 | + | return | |
| 263 | + | } | |
| 264 | + | w.putUint8(0) | |
| 265 | + | } | |
| 266 | + | ||
| 257 | 267 | func (w *payloadWriter) putUint16(v uint16) { | |
| 258 | 268 | var tmp [2]byte | |
| 259 | 269 | binary.BigEndian.PutUint16(tmp[:], v) | |
| @@ -403,11 +413,7 @@ func MarshalHello(h Hello) []byte { | |||
| 403 | 413 | w := payloadWriter{} | |
| 404 | 414 | w.putUint16(h.Version) | |
| 405 | 415 | w.putUint8(h.Intent) | |
| 406 | - | if h.Proxied { | |
| 407 | - | w.putUint8(1) | |
| 408 | - | } else { | |
| 409 | - | w.putUint8(0) | |
| 410 | - | } | |
| 416 | + | w.putBool(h.Proxied) | |
| 411 | 417 | w.putBytes(h.ClientID[:]) | |
| 412 | 418 | w.putUint64(h.ResumeToken) | |
| 413 | 419 | w.putString(h.Name) | |
| @@ -415,11 +421,7 @@ func MarshalHello(h Hello) []byte { | |||
| 415 | 421 | w.putUint16(uint16(h.Size.Rows)) | |
| 416 | 422 | w.putString(h.TermEnv) | |
| 417 | 423 | w.putString(h.Cwd) | |
| 418 | - | if h.TrueColor { | |
| 419 | - | w.putUint8(1) | |
| 420 | - | } else { | |
| 421 | - | w.putUint8(0) | |
| 422 | - | } | |
| 424 | + | w.putBool(h.TrueColor) | |
| 423 | 425 | w.putUint8(h.MaxOutputInFlight) | |
| 424 | 426 | w.putUint32(uint32(len(h.Env))) | |
| 425 | 427 | for _, entry := range h.Env { | |
| @@ -926,11 +928,7 @@ func MarshalSessionMeta(m SessionMeta) ([]byte, error) { | |||
| 926 | 928 | for _, tab := range m.Tabs { | |
| 927 | 929 | w.putUint16(tab.Index) | |
| 928 | 930 | w.putString(tab.Name) | |
| 929 | - | if tab.Attention { | |
| 930 | - | w.putUint8(1) | |
| 931 | - | } else { | |
| 932 | - | w.putUint8(0) | |
| 933 | - | } | |
| 931 | + | w.putBool(tab.Attention) | |
| 934 | 932 | } | |
| 935 | 933 | return w.b, nil | |
| 936 | 934 | } | |
| @@ -254,6 +254,16 @@ func (w *payloadWriter) putUint8(v uint8) { | |||||
| 254 | w.b = append(w.b, v) | 254 | w.b = append(w.b, v) | ||
| 255 | } | 255 | } | ||
| 256 | 256 | ||||
| 257 | + | // putBool writes the single byte payloadReader.getBool accepts: 1 for true and | |||
| 258 | + | // 0 for false. | |||
| 259 | + | func (w *payloadWriter) putBool(v bool) { | |||
| 260 | + | if v { | |||
| 261 | + | w.putUint8(1) | |||
| 262 | + | return | |||
| 263 | + | } | |||
| 264 | + | w.putUint8(0) | |||
| 265 | + | } | |||
| 266 | + | ||||
| 257 | func (w *payloadWriter) putUint16(v uint16) { | 267 | func (w *payloadWriter) putUint16(v uint16) { | ||
| 258 | var tmp [2]byte | 268 | var tmp [2]byte | ||
| 259 | binary.BigEndian.PutUint16(tmp[:], v) | 269 | binary.BigEndian.PutUint16(tmp[:], v) | ||
| @@ -403,11 +413,7 @@ func MarshalHello(h Hello) []byte { | |||||
| 403 | w := payloadWriter{} | 413 | w := payloadWriter{} | ||
| 404 | w.putUint16(h.Version) | 414 | w.putUint16(h.Version) | ||
| 405 | w.putUint8(h.Intent) | 415 | w.putUint8(h.Intent) | ||
| 406 | - | if h.Proxied { | 416 | + | w.putBool(h.Proxied) |
| 407 | - | w.putUint8(1) | |||
| 408 | - | } else { | |||
| 409 | - | w.putUint8(0) | |||
| 410 | - | } | |||
| 411 | w.putBytes(h.ClientID[:]) | 417 | w.putBytes(h.ClientID[:]) | ||
| 412 | w.putUint64(h.ResumeToken) | 418 | w.putUint64(h.ResumeToken) | ||
| 413 | w.putString(h.Name) | 419 | w.putString(h.Name) | ||
| @@ -415,11 +421,7 @@ func MarshalHello(h Hello) []byte { | |||||
| 415 | w.putUint16(uint16(h.Size.Rows)) | 421 | w.putUint16(uint16(h.Size.Rows)) | ||
| 416 | w.putString(h.TermEnv) | 422 | w.putString(h.TermEnv) | ||
| 417 | w.putString(h.Cwd) | 423 | w.putString(h.Cwd) | ||
| 418 | - | if h.TrueColor { | 424 | + | w.putBool(h.TrueColor) |
| 419 | - | w.putUint8(1) | |||
| 420 | - | } else { | |||
| 421 | - | w.putUint8(0) | |||
| 422 | - | } | |||
| 423 | w.putUint8(h.MaxOutputInFlight) | 425 | w.putUint8(h.MaxOutputInFlight) | ||
| 424 | w.putUint32(uint32(len(h.Env))) | 426 | w.putUint32(uint32(len(h.Env))) | ||
| 425 | for _, entry := range h.Env { | 427 | for _, entry := range h.Env { | ||
| @@ -926,11 +928,7 @@ func MarshalSessionMeta(m SessionMeta) ([]byte, error) { | |||||
| 926 | for _, tab := range m.Tabs { | 928 | for _, tab := range m.Tabs { | ||
| 927 | w.putUint16(tab.Index) | 929 | w.putUint16(tab.Index) | ||
| 928 | w.putString(tab.Name) | 930 | w.putString(tab.Name) | ||
| 929 | - | if tab.Attention { | 931 | + | w.putBool(tab.Attention) |
| 930 | - | w.putUint8(1) | |||
| 931 | - | } else { | |||
| 932 | - | w.putUint8(0) | |||
| 933 | - | } | |||
| 934 | } | 932 | } | ||
| 935 | return w.b, nil | 933 | return w.b, nil | ||
| 936 | } | 934 | } | ||
M internal/ports/wire_command.go +4 -20
| @@ -51,16 +51,8 @@ func MarshalCommandRequest(m CommandRequest) ([]byte, error) { | |||
| 51 | 51 | w := payloadWriter{} | |
| 52 | 52 | w.putUint16(m.Version) | |
| 53 | 53 | w.putUint64(m.RequestID) | |
| 54 | - | if m.Attached { | |
| 55 | - | w.putUint8(1) | |
| 56 | - | } else { | |
| 57 | - | w.putUint8(0) | |
| 58 | - | } | |
| 59 | - | if m.Self { | |
| 60 | - | w.putUint8(1) | |
| 61 | - | } else { | |
| 62 | - | w.putUint8(0) | |
| 63 | - | } | |
| 54 | + | w.putBool(m.Attached) | |
| 55 | + | w.putBool(m.Self) | |
| 64 | 56 | w.putString(m.Slug) | |
| 65 | 57 | w.putUint16(uint16(len(m.Args))) | |
| 66 | 58 | for _, arg := range m.Args { | |
| @@ -69,11 +61,7 @@ func MarshalCommandRequest(m CommandRequest) ([]byte, error) { | |||
| 69 | 61 | w.putString(m.TargetSession) | |
| 70 | 62 | w.putString(m.TargetTab) | |
| 71 | 63 | w.putString(m.TargetPane) | |
| 72 | - | if m.JSON { | |
| 73 | - | w.putUint8(1) | |
| 74 | - | } else { | |
| 75 | - | w.putUint8(0) | |
| 76 | - | } | |
| 64 | + | w.putBool(m.JSON) | |
| 77 | 65 | return w.b, nil | |
| 78 | 66 | } | |
| 79 | 67 | ||
| @@ -139,11 +127,7 @@ func UnmarshalCommandRequest(b []byte) (CommandRequest, error) { | |||
| 139 | 127 | func MarshalCommandResult(m CommandResult) []byte { | |
| 140 | 128 | w := payloadWriter{} | |
| 141 | 129 | w.putUint64(m.RequestID) | |
| 142 | - | if m.OK { | |
| 143 | - | w.putUint8(1) | |
| 144 | - | } else { | |
| 145 | - | w.putUint8(0) | |
| 146 | - | } | |
| 130 | + | w.putBool(m.OK) | |
| 147 | 131 | w.putUint16(m.Code) | |
| 148 | 132 | w.putString(m.Text) | |
| 149 | 133 | w.putLongString(m.Output) | |
| @@ -51,16 +51,8 @@ func MarshalCommandRequest(m CommandRequest) ([]byte, error) { | |||||
| 51 | w := payloadWriter{} | 51 | w := payloadWriter{} | ||
| 52 | w.putUint16(m.Version) | 52 | w.putUint16(m.Version) | ||
| 53 | w.putUint64(m.RequestID) | 53 | w.putUint64(m.RequestID) | ||
| 54 | - | if m.Attached { | 54 | + | w.putBool(m.Attached) |
| 55 | - | w.putUint8(1) | 55 | + | w.putBool(m.Self) |
| 56 | - | } else { | |||
| 57 | - | w.putUint8(0) | |||
| 58 | - | } | |||
| 59 | - | if m.Self { | |||
| 60 | - | w.putUint8(1) | |||
| 61 | - | } else { | |||
| 62 | - | w.putUint8(0) | |||
| 63 | - | } | |||
| 64 | w.putString(m.Slug) | 56 | w.putString(m.Slug) | ||
| 65 | w.putUint16(uint16(len(m.Args))) | 57 | w.putUint16(uint16(len(m.Args))) | ||
| 66 | for _, arg := range m.Args { | 58 | for _, arg := range m.Args { | ||
| @@ -69,11 +61,7 @@ func MarshalCommandRequest(m CommandRequest) ([]byte, error) { | |||||
| 69 | w.putString(m.TargetSession) | 61 | w.putString(m.TargetSession) | ||
| 70 | w.putString(m.TargetTab) | 62 | w.putString(m.TargetTab) | ||
| 71 | w.putString(m.TargetPane) | 63 | w.putString(m.TargetPane) | ||
| 72 | - | if m.JSON { | 64 | + | w.putBool(m.JSON) |
| 73 | - | w.putUint8(1) | |||
| 74 | - | } else { | |||
| 75 | - | w.putUint8(0) | |||
| 76 | - | } | |||
| 77 | return w.b, nil | 65 | return w.b, nil | ||
| 78 | } | 66 | } | ||
| 79 | 67 | ||||
| @@ -139,11 +127,7 @@ func UnmarshalCommandRequest(b []byte) (CommandRequest, error) { | |||||
| 139 | func MarshalCommandResult(m CommandResult) []byte { | 127 | func MarshalCommandResult(m CommandResult) []byte { | ||
| 140 | w := payloadWriter{} | 128 | w := payloadWriter{} | ||
| 141 | w.putUint64(m.RequestID) | 129 | w.putUint64(m.RequestID) | ||
| 142 | - | if m.OK { | 130 | + | w.putBool(m.OK) |
| 143 | - | w.putUint8(1) | |||
| 144 | - | } else { | |||
| 145 | - | w.putUint8(0) | |||
| 146 | - | } | |||
| 147 | w.putUint16(m.Code) | 131 | w.putUint16(m.Code) | ||
| 148 | w.putString(m.Text) | 132 | w.putString(m.Text) | ||
| 149 | w.putLongString(m.Output) | 133 | w.putLongString(m.Output) | ||
M internal/usecase/daemon/client.go +20 -2
| @@ -307,11 +307,29 @@ func (ac *attachedClient) sendExpectedTransport(expected transportSnapshot, f po | |||
| 307 | 307 | return expected.transport.Send(f) | |
| 308 | 308 | } | |
| 309 | 309 | ||
| 310 | + | // beginExpectedTransportSendLocked validates that expected is still the | |
| 311 | + | // attachment's current transport incarnation and admits ticket's interruptible | |
| 312 | + | // transport send. The caller must already hold ac.sendMu and, on success, owns | |
| 313 | + | // both the send and its matching ticket.endTransportSend. | |
| 314 | + | func (ac *attachedClient) beginExpectedTransportSendLocked(expected transportSnapshot, ticket *roleEffectTicket) error { | |
| 315 | + | if expected.transport == nil || !ac.transportSnapshotCurrent(expected) { | |
| 316 | + | return errTransportReplaced | |
| 317 | + | } | |
| 318 | + | if ticket != nil && (ticket.ended.Load() || !ticket.beginTransportSend(expected)) { | |
| 319 | + | return errAttachmentTransition | |
| 320 | + | } | |
| 321 | + | return nil | |
| 322 | + | } | |
| 323 | + | ||
| 310 | 324 | func (ac *attachedClient) sendExpectedTransportForRole(expected transportSnapshot, f ports.Frame, ticket *roleEffectTicket) error { | |
| 311 | 325 | ac.sendMu.Lock() | |
| 312 | 326 | defer ac.sendMu.Unlock() | |
| 313 | - | if ticket == nil || ticket.ended.Load() || !ac.transportSnapshotCurrent(expected) || | |
| 314 | - | expected.transport == nil || !ticket.beginTransportSend(expected) { | |
| 327 | + | // A role-bound send requires a live ticket, and reports every rejection, | |
| 328 | + | // including a replaced transport, as a lost attachment transition. | |
| 329 | + | if ticket == nil { | |
| 330 | + | return errAttachmentTransition | |
| 331 | + | } | |
| 332 | + | if err := ac.beginExpectedTransportSendLocked(expected, ticket); err != nil { | |
| 315 | 333 | return errAttachmentTransition | |
| 316 | 334 | } | |
| 317 | 335 | err := expected.transport.Send(f) | |
| @@ -307,11 +307,29 @@ func (ac *attachedClient) sendExpectedTransport(expected transportSnapshot, f po | |||||
| 307 | return expected.transport.Send(f) | 307 | return expected.transport.Send(f) | ||
| 308 | } | 308 | } | ||
| 309 | 309 | ||||
| 310 | + | // beginExpectedTransportSendLocked validates that expected is still the | |||
| 311 | + | // attachment's current transport incarnation and admits ticket's interruptible | |||
| 312 | + | // transport send. The caller must already hold ac.sendMu and, on success, owns | |||
| 313 | + | // both the send and its matching ticket.endTransportSend. | |||
| 314 | + | func (ac *attachedClient) beginExpectedTransportSendLocked(expected transportSnapshot, ticket *roleEffectTicket) error { | |||
| 315 | + | if expected.transport == nil || !ac.transportSnapshotCurrent(expected) { | |||
| 316 | + | return errTransportReplaced | |||
| 317 | + | } | |||
| 318 | + | if ticket != nil && (ticket.ended.Load() || !ticket.beginTransportSend(expected)) { | |||
| 319 | + | return errAttachmentTransition | |||
| 320 | + | } | |||
| 321 | + | return nil | |||
| 322 | + | } | |||
| 323 | + | ||||
| 310 | func (ac *attachedClient) sendExpectedTransportForRole(expected transportSnapshot, f ports.Frame, ticket *roleEffectTicket) error { | 324 | func (ac *attachedClient) sendExpectedTransportForRole(expected transportSnapshot, f ports.Frame, ticket *roleEffectTicket) error { | ||
| 311 | ac.sendMu.Lock() | 325 | ac.sendMu.Lock() | ||
| 312 | defer ac.sendMu.Unlock() | 326 | defer ac.sendMu.Unlock() | ||
| 313 | - | if ticket == nil || ticket.ended.Load() || !ac.transportSnapshotCurrent(expected) || | 327 | + | // A role-bound send requires a live ticket, and reports every rejection, |
| 314 | - | expected.transport == nil || !ticket.beginTransportSend(expected) { | 328 | + | // including a replaced transport, as a lost attachment transition. |
| 329 | + | if ticket == nil { | |||
| 330 | + | return errAttachmentTransition | |||
| 331 | + | } | |||
| 332 | + | if err := ac.beginExpectedTransportSendLocked(expected, ticket); err != nil { | |||
| 315 | return errAttachmentTransition | 333 | return errAttachmentTransition | ||
| 316 | } | 334 | } | ||
| 317 | err := expected.transport.Send(f) | 335 | err := expected.transport.Send(f) | ||
M internal/usecase/daemon/proxied_contract_test.go +48 -26
| @@ -79,7 +79,9 @@ func TestOrdinaryHandshakeRetainsChromeAndNoMetadata(t *testing.T) { | |||
| 79 | 79 | require.NoError(t, err) | |
| 80 | 80 | require.Equal(t, ports.CapabilityResume, welcome.Capabilities) | |
| 81 | 81 | ||
| 82 | - | outputFrame := awaitFrame(t, sends, ports.MsgOutput) | |
| 82 | + | outputFrame := awaitTestValue(t, sends, "ordinary handshake produced no post-welcome frame") | |
| 83 | + | require.NotEqual(t, ports.MsgSessionMeta, outputFrame.Type, "an ordinary attachment must never receive session metadata") | |
| 84 | + | require.Equal(t, ports.MsgOutput, outputFrame.Type) | |
| 83 | 85 | output, err := ports.UnmarshalOutput(outputFrame.Payload) | |
| 84 | 86 | require.NoError(t, err) | |
| 85 | 87 | require.Zero(t, output.BaseStateNum) | |
| @@ -146,31 +148,44 @@ func TestProxiedTransactionalResizeUsesReceivedContentGeometry(t *testing.T) { | |||
| 146 | 148 | } | |
| 147 | 149 | ||
| 148 | 150 | func TestProxiedModeCannotChangeAcrossResume(t *testing.T) { | |
| 149 | - | d := newTestDaemon(t, newFactory(t, newQuietPTY()), stubClock{}) | |
| 150 | - | var clientID [16]byte | |
| 151 | - | clientID[0] = 1 | |
| 152 | - | originalTransport := &closeTrackingTransport{} | |
| 153 | - | hello := ports.Hello{ | |
| 154 | - | Version: ports.ProtocolVersion, Intent: ports.IntentNew, Proxied: true, | |
| 155 | - | ClientID: clientID, Name: "immutable", Size: domain.Size{Cols: 40, Rows: 8}, | |
| 151 | + | tests := []struct { | |
| 152 | + | name string | |
| 153 | + | initialProxied bool | |
| 154 | + | resumedProxied bool | |
| 155 | + | }{ | |
| 156 | + | {name: "proxied attachment cannot resume as ordinary", initialProxied: true, resumedProxied: false}, | |
| 157 | + | {name: "ordinary attachment cannot resume as proxied", initialProxied: false, resumedProxied: true}, | |
| 158 | + | } | |
| 159 | + | for _, tt := range tests { | |
| 160 | + | t.Run(tt.name, func(t *testing.T) { | |
| 161 | + | d := newTestDaemon(t, newFactory(t, newQuietPTY()), stubClock{}) | |
| 162 | + | var clientID [16]byte | |
| 163 | + | clientID[0] = 1 | |
| 164 | + | originalTransport := &closeTrackingTransport{} | |
| 165 | + | hello := ports.Hello{ | |
| 166 | + | Version: ports.ProtocolVersion, Intent: ports.IntentNew, Proxied: tt.initialProxied, | |
| 167 | + | ClientID: clientID, Name: "immutable", Size: domain.Size{Cols: 40, Rows: 8}, | |
| 168 | + | } | |
| 169 | + | sess, ac, err := d.route(hello, originalTransport) | |
| 170 | + | require.NoError(t, err) | |
| 171 | + | token := ac.resumeToken | |
| 172 | + | d.clientGone(sess, ac, originalTransport, false) | |
| 173 | + | ||
| 174 | + | resume := hello | |
| 175 | + | resume.Intent = ports.IntentResume | |
| 176 | + | resume.ResumeToken = token | |
| 177 | + | resume.Proxied = tt.resumedProxied | |
| 178 | + | _, resumed, ok, err := d.resumeParked(resume, &closeTrackingTransport{}, resume.Size) | |
| 179 | + | require.Error(t, err) | |
| 180 | + | require.False(t, ok) | |
| 181 | + | require.Nil(t, resumed) | |
| 182 | + | require.Equal(t, tt.initialProxied, ac.proxied) | |
| 183 | + | d.mu.Lock() | |
| 184 | + | parked := d.parked[token] | |
| 185 | + | d.mu.Unlock() | |
| 186 | + | require.NotNil(t, parked, "a mode-mismatched resume must leave the original attachment parked") | |
| 187 | + | }) | |
| 156 | 188 | } | |
| 157 | - | sess, ac, err := d.route(hello, originalTransport) | |
| 158 | - | require.NoError(t, err) | |
| 159 | - | token := ac.resumeToken | |
| 160 | - | d.clientGone(sess, ac, originalTransport, false) | |
| 161 | - | ||
| 162 | - | resume := hello | |
| 163 | - | resume.Intent = ports.IntentResume | |
| 164 | - | resume.ResumeToken = token | |
| 165 | - | resume.Proxied = false | |
| 166 | - | _, resumed, ok, err := d.resumeParked(resume, &closeTrackingTransport{}, resume.Size) | |
| 167 | - | require.Error(t, err) | |
| 168 | - | require.False(t, ok) | |
| 169 | - | require.Nil(t, resumed) | |
| 170 | - | require.True(t, ac.proxied) | |
| 171 | - | d.mu.Lock() | |
| 172 | - | require.NotNil(t, d.parked[token], "a mode-mismatched resume must leave the original attachment parked") | |
| 173 | - | d.mu.Unlock() | |
| 174 | 189 | } | |
| 175 | 190 | ||
| 176 | 191 | func TestOutputResetRebasesFullWindowAndSchedulesBaseZeroPaint(t *testing.T) { | |
| @@ -218,14 +233,19 @@ func TestOutputResetRequiresProxiedActiveRoleAndStrictPayload(t *testing.T) { | |||
| 218 | 233 | d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 219 | 234 | ac.proxied = tt.proxied | |
| 220 | 235 | d.attachCoordinator(sess, nil, ac, true) | |
| 236 | + | ac.sendMu.Lock() | |
| 221 | 237 | ac.output.next, ac.output.acked = 4, 0 | |
| 238 | + | ac.sendMu.Unlock() | |
| 222 | 239 | ||
| 223 | 240 | token := sess.attachmentToken(ac, ac.transport()) | |
| 224 | 241 | if tt.staleRole { | |
| 225 | 242 | token.role = attachmentSnatched | |
| 226 | 243 | } | |
| 227 | 244 | require.False(t, d.handleActiveClientFrame(token, ports.Frame{Type: ports.MsgOutputResetRequest, Payload: tt.payload})) | |
| 228 | - | require.Zero(t, ac.output.acked) | |
| 245 | + | ac.sendMu.Lock() | |
| 246 | + | acked := ac.output.acked | |
| 247 | + | ac.sendMu.Unlock() | |
| 248 | + | require.Zero(t, acked) | |
| 229 | 249 | }) | |
| 230 | 250 | } | |
| 231 | 251 | } | |
| @@ -234,7 +254,9 @@ func TestOutputResetRevalidatesTransportUnderSendLock(t *testing.T) { | |||
| 234 | 254 | d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 235 | 255 | ac.proxied = true | |
| 236 | 256 | d.attachCoordinator(sess, nil, ac, true) | |
| 257 | + | ac.sendMu.Lock() | |
| 237 | 258 | ac.output.next, ac.output.acked = 4, 0 | |
| 259 | + | ac.sendMu.Unlock() | |
| 238 | 260 | token := sess.attachmentToken(ac, ac.transport()) | |
| 239 | 261 | ||
| 240 | 262 | admitted := make(chan struct{}) | |
| @@ -79,7 +79,9 @@ func TestOrdinaryHandshakeRetainsChromeAndNoMetadata(t *testing.T) { | |||||
| 79 | require.NoError(t, err) | 79 | require.NoError(t, err) | ||
| 80 | require.Equal(t, ports.CapabilityResume, welcome.Capabilities) | 80 | require.Equal(t, ports.CapabilityResume, welcome.Capabilities) | ||
| 81 | 81 | ||||
| 82 | - | outputFrame := awaitFrame(t, sends, ports.MsgOutput) | 82 | + | outputFrame := awaitTestValue(t, sends, "ordinary handshake produced no post-welcome frame") |
| 83 | + | require.NotEqual(t, ports.MsgSessionMeta, outputFrame.Type, "an ordinary attachment must never receive session metadata") | |||
| 84 | + | require.Equal(t, ports.MsgOutput, outputFrame.Type) | |||
| 83 | output, err := ports.UnmarshalOutput(outputFrame.Payload) | 85 | output, err := ports.UnmarshalOutput(outputFrame.Payload) | ||
| 84 | require.NoError(t, err) | 86 | require.NoError(t, err) | ||
| 85 | require.Zero(t, output.BaseStateNum) | 87 | require.Zero(t, output.BaseStateNum) | ||
| @@ -146,31 +148,44 @@ func TestProxiedTransactionalResizeUsesReceivedContentGeometry(t *testing.T) { | |||||
| 146 | } | 148 | } | ||
| 147 | 149 | ||||
| 148 | func TestProxiedModeCannotChangeAcrossResume(t *testing.T) { | 150 | func TestProxiedModeCannotChangeAcrossResume(t *testing.T) { | ||
| 149 | - | d := newTestDaemon(t, newFactory(t, newQuietPTY()), stubClock{}) | 151 | + | tests := []struct { |
| 150 | - | var clientID [16]byte | 152 | + | name string |
| 151 | - | clientID[0] = 1 | 153 | + | initialProxied bool |
| 152 | - | originalTransport := &closeTrackingTransport{} | 154 | + | resumedProxied bool |
| 153 | - | hello := ports.Hello{ | 155 | + | }{ |
| 154 | - | Version: ports.ProtocolVersion, Intent: ports.IntentNew, Proxied: true, | 156 | + | {name: "proxied attachment cannot resume as ordinary", initialProxied: true, resumedProxied: false}, |
| 155 | - | ClientID: clientID, Name: "immutable", Size: domain.Size{Cols: 40, Rows: 8}, | 157 | + | {name: "ordinary attachment cannot resume as proxied", initialProxied: false, resumedProxied: true}, |
| 158 | + | } | |||
| 159 | + | for _, tt := range tests { | |||
| 160 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 161 | + | d := newTestDaemon(t, newFactory(t, newQuietPTY()), stubClock{}) | |||
| 162 | + | var clientID [16]byte | |||
| 163 | + | clientID[0] = 1 | |||
| 164 | + | originalTransport := &closeTrackingTransport{} | |||
| 165 | + | hello := ports.Hello{ | |||
| 166 | + | Version: ports.ProtocolVersion, Intent: ports.IntentNew, Proxied: tt.initialProxied, | |||
| 167 | + | ClientID: clientID, Name: "immutable", Size: domain.Size{Cols: 40, Rows: 8}, | |||
| 168 | + | } | |||
| 169 | + | sess, ac, err := d.route(hello, originalTransport) | |||
| 170 | + | require.NoError(t, err) | |||
| 171 | + | token := ac.resumeToken | |||
| 172 | + | d.clientGone(sess, ac, originalTransport, false) | |||
| 173 | + | ||||
| 174 | + | resume := hello | |||
| 175 | + | resume.Intent = ports.IntentResume | |||
| 176 | + | resume.ResumeToken = token | |||
| 177 | + | resume.Proxied = tt.resumedProxied | |||
| 178 | + | _, resumed, ok, err := d.resumeParked(resume, &closeTrackingTransport{}, resume.Size) | |||
| 179 | + | require.Error(t, err) | |||
| 180 | + | require.False(t, ok) | |||
| 181 | + | require.Nil(t, resumed) | |||
| 182 | + | require.Equal(t, tt.initialProxied, ac.proxied) | |||
| 183 | + | d.mu.Lock() | |||
| 184 | + | parked := d.parked[token] | |||
| 185 | + | d.mu.Unlock() | |||
| 186 | + | require.NotNil(t, parked, "a mode-mismatched resume must leave the original attachment parked") | |||
| 187 | + | }) | |||
| 156 | } | 188 | } | ||
| 157 | - | sess, ac, err := d.route(hello, originalTransport) | |||
| 158 | - | require.NoError(t, err) | |||
| 159 | - | token := ac.resumeToken | |||
| 160 | - | d.clientGone(sess, ac, originalTransport, false) | |||
| 161 | - | ||||
| 162 | - | resume := hello | |||
| 163 | - | resume.Intent = ports.IntentResume | |||
| 164 | - | resume.ResumeToken = token | |||
| 165 | - | resume.Proxied = false | |||
| 166 | - | _, resumed, ok, err := d.resumeParked(resume, &closeTrackingTransport{}, resume.Size) | |||
| 167 | - | require.Error(t, err) | |||
| 168 | - | require.False(t, ok) | |||
| 169 | - | require.Nil(t, resumed) | |||
| 170 | - | require.True(t, ac.proxied) | |||
| 171 | - | d.mu.Lock() | |||
| 172 | - | require.NotNil(t, d.parked[token], "a mode-mismatched resume must leave the original attachment parked") | |||
| 173 | - | d.mu.Unlock() | |||
| 174 | } | 189 | } | ||
| 175 | 190 | ||||
| 176 | func TestOutputResetRebasesFullWindowAndSchedulesBaseZeroPaint(t *testing.T) { | 191 | func TestOutputResetRebasesFullWindowAndSchedulesBaseZeroPaint(t *testing.T) { | ||
| @@ -218,14 +233,19 @@ func TestOutputResetRequiresProxiedActiveRoleAndStrictPayload(t *testing.T) { | |||||
| 218 | d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | 233 | d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | ||
| 219 | ac.proxied = tt.proxied | 234 | ac.proxied = tt.proxied | ||
| 220 | d.attachCoordinator(sess, nil, ac, true) | 235 | d.attachCoordinator(sess, nil, ac, true) | ||
| 236 | + | ac.sendMu.Lock() | |||
| 221 | ac.output.next, ac.output.acked = 4, 0 | 237 | ac.output.next, ac.output.acked = 4, 0 | ||
| 238 | + | ac.sendMu.Unlock() | |||
| 222 | 239 | ||||
| 223 | token := sess.attachmentToken(ac, ac.transport()) | 240 | token := sess.attachmentToken(ac, ac.transport()) | ||
| 224 | if tt.staleRole { | 241 | if tt.staleRole { | ||
| 225 | token.role = attachmentSnatched | 242 | token.role = attachmentSnatched | ||
| 226 | } | 243 | } | ||
| 227 | require.False(t, d.handleActiveClientFrame(token, ports.Frame{Type: ports.MsgOutputResetRequest, Payload: tt.payload})) | 244 | require.False(t, d.handleActiveClientFrame(token, ports.Frame{Type: ports.MsgOutputResetRequest, Payload: tt.payload})) | ||
| 228 | - | require.Zero(t, ac.output.acked) | 245 | + | ac.sendMu.Lock() |
| 246 | + | acked := ac.output.acked | |||
| 247 | + | ac.sendMu.Unlock() | |||
| 248 | + | require.Zero(t, acked) | |||
| 229 | }) | 249 | }) | ||
| 230 | } | 250 | } | ||
| 231 | } | 251 | } | ||
| @@ -234,7 +254,9 @@ func TestOutputResetRevalidatesTransportUnderSendLock(t *testing.T) { | |||||
| 234 | d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | 254 | d, sess, ac, _ := newManualSessionWithPTYs(t, newQuietPTY()) | ||
| 235 | ac.proxied = true | 255 | ac.proxied = true | ||
| 236 | d.attachCoordinator(sess, nil, ac, true) | 256 | d.attachCoordinator(sess, nil, ac, true) | ||
| 257 | + | ac.sendMu.Lock() | |||
| 237 | ac.output.next, ac.output.acked = 4, 0 | 258 | ac.output.next, ac.output.acked = 4, 0 | ||
| 259 | + | ac.sendMu.Unlock() | |||
| 238 | token := sess.attachmentToken(ac, ac.transport()) | 260 | token := sess.attachmentToken(ac, ac.transport()) | ||
| 239 | 261 | ||||
| 240 | admitted := make(chan struct{}) | 262 | admitted := make(chan struct{}) | ||
M internal/usecase/daemon/render_pipeline.go +20 -16
| @@ -485,31 +485,35 @@ func (d *Daemon) emitFrame(sess *session, ac *attachedClient, state *capturedRen | |||
| 485 | 485 | data = append(data, ac.encodeCursorTail(composed.cursor, len(data) > 0)...) | |
| 486 | 486 | var sendTr ports.Transport | |
| 487 | 487 | var sendErr error | |
| 488 | - | if len(data) > 0 { | |
| 488 | + | // Metadata precedes any output bytes and is published on its own when the | |
| 489 | + | // terminal frame is empty, so a proxied client never keeps a stale snapshot. | |
| 490 | + | if len(data) > 0 || ac.proxied { | |
| 489 | 491 | sendTransport := ac.transportSnapshot() | |
| 490 | 492 | sendTr = sendTransport.transport | |
| 491 | 493 | if ac.proxied { | |
| 492 | 494 | sendErr = ac.sendSessionMetaIfChanged(sess, sendTransport, marks.roleEffect) | |
| 493 | 495 | } | |
| 494 | - | endEmit := marks.span(ports.RuntimeEmitStart, ports.RuntimeEmitEnd, uint64(len(data))) | |
| 495 | - | if sendErr == nil && sendTr == nil { | |
| 496 | - | sendErr = errors.New("client transport is nil") | |
| 497 | - | } | |
| 498 | - | if sendErr == nil { | |
| 499 | - | send := sendTr.Send | |
| 500 | - | if async, ok := sendTr.(ports.AsyncTransport); ok { | |
| 501 | - | send = async.SendAsync | |
| 496 | + | if len(data) > 0 { | |
| 497 | + | endEmit := marks.span(ports.RuntimeEmitStart, ports.RuntimeEmitEnd, uint64(len(data))) | |
| 498 | + | if sendErr == nil && sendTr == nil { | |
| 499 | + | sendErr = errors.New("client transport is nil") | |
| 502 | 500 | } | |
| 503 | - | interruptible := marks.roleEffect != nil && marks.roleEffect.beginTransportSend(sendTransport) | |
| 504 | - | sendErr = prepared.send(data, ac.echoAck.Load(), send) | |
| 505 | - | if interruptible { | |
| 506 | - | if sendErr != nil { | |
| 507 | - | marks.roleEffect.reportTransportFailure(sendTransport) | |
| 501 | + | if sendErr == nil { | |
| 502 | + | send := sendTr.Send | |
| 503 | + | if async, ok := sendTr.(ports.AsyncTransport); ok { | |
| 504 | + | send = async.SendAsync | |
| 505 | + | } | |
| 506 | + | interruptible := marks.roleEffect != nil && marks.roleEffect.beginTransportSend(sendTransport) | |
| 507 | + | sendErr = prepared.send(data, ac.echoAck.Load(), send) | |
| 508 | + | if interruptible { | |
| 509 | + | if sendErr != nil { | |
| 510 | + | marks.roleEffect.reportTransportFailure(sendTransport) | |
| 511 | + | } | |
| 512 | + | marks.roleEffect.endTransportSend() | |
| 508 | 513 | } | |
| 509 | - | marks.roleEffect.endTransportSend() | |
| 510 | 514 | } | |
| 515 | + | endEmit(uint64(len(data)), sendErr == nil) | |
| 511 | 516 | } | |
| 512 | - | endEmit(uint64(len(data)), sendErr == nil) | |
| 513 | 517 | } | |
| 514 | 518 | if sendErr == nil { | |
| 515 | 519 | // Publish only after output preparation and transport emission both | |
| @@ -485,31 +485,35 @@ func (d *Daemon) emitFrame(sess *session, ac *attachedClient, state *capturedRen | |||||
| 485 | data = append(data, ac.encodeCursorTail(composed.cursor, len(data) > 0)...) | 485 | data = append(data, ac.encodeCursorTail(composed.cursor, len(data) > 0)...) | ||
| 486 | var sendTr ports.Transport | 486 | var sendTr ports.Transport | ||
| 487 | var sendErr error | 487 | var sendErr error | ||
| 488 | - | if len(data) > 0 { | 488 | + | // Metadata precedes any output bytes and is published on its own when the |
| 489 | + | // terminal frame is empty, so a proxied client never keeps a stale snapshot. | |||
| 490 | + | if len(data) > 0 || ac.proxied { | |||
| 489 | sendTransport := ac.transportSnapshot() | 491 | sendTransport := ac.transportSnapshot() | ||
| 490 | sendTr = sendTransport.transport | 492 | sendTr = sendTransport.transport | ||
| 491 | if ac.proxied { | 493 | if ac.proxied { | ||
| 492 | sendErr = ac.sendSessionMetaIfChanged(sess, sendTransport, marks.roleEffect) | 494 | sendErr = ac.sendSessionMetaIfChanged(sess, sendTransport, marks.roleEffect) | ||
| 493 | } | 495 | } | ||
| 494 | - | endEmit := marks.span(ports.RuntimeEmitStart, ports.RuntimeEmitEnd, uint64(len(data))) | 496 | + | if len(data) > 0 { |
| 495 | - | if sendErr == nil && sendTr == nil { | 497 | + | endEmit := marks.span(ports.RuntimeEmitStart, ports.RuntimeEmitEnd, uint64(len(data))) |
| 496 | - | sendErr = errors.New("client transport is nil") | 498 | + | if sendErr == nil && sendTr == nil { |
| 497 | - | } | 499 | + | sendErr = errors.New("client transport is nil") |
| 498 | - | if sendErr == nil { | |||
| 499 | - | send := sendTr.Send | |||
| 500 | - | if async, ok := sendTr.(ports.AsyncTransport); ok { | |||
| 501 | - | send = async.SendAsync | |||
| 502 | } | 500 | } | ||
| 503 | - | interruptible := marks.roleEffect != nil && marks.roleEffect.beginTransportSend(sendTransport) | 501 | + | if sendErr == nil { |
| 504 | - | sendErr = prepared.send(data, ac.echoAck.Load(), send) | 502 | + | send := sendTr.Send |
| 505 | - | if interruptible { | 503 | + | if async, ok := sendTr.(ports.AsyncTransport); ok { |
| 506 | - | if sendErr != nil { | 504 | + | send = async.SendAsync |
| 507 | - | marks.roleEffect.reportTransportFailure(sendTransport) | 505 | + | } |
| 506 | + | interruptible := marks.roleEffect != nil && marks.roleEffect.beginTransportSend(sendTransport) | |||
| 507 | + | sendErr = prepared.send(data, ac.echoAck.Load(), send) | |||
| 508 | + | if interruptible { | |||
| 509 | + | if sendErr != nil { | |||
| 510 | + | marks.roleEffect.reportTransportFailure(sendTransport) | |||
| 511 | + | } | |||
| 512 | + | marks.roleEffect.endTransportSend() | |||
| 508 | } | 513 | } | ||
| 509 | - | marks.roleEffect.endTransportSend() | |||
| 510 | } | 514 | } | ||
| 515 | + | endEmit(uint64(len(data)), sendErr == nil) | |||
| 511 | } | 516 | } | ||
| 512 | - | endEmit(uint64(len(data)), sendErr == nil) | |||
| 513 | } | 517 | } | ||
| 514 | if sendErr == nil { | 518 | if sendErr == nil { | ||
| 515 | // Publish only after output preparation and transport emission both | 519 | // Publish only after output preparation and transport emission both | ||
M internal/usecase/daemon/session_meta.go +5 -6
| @@ -6,6 +6,8 @@ import ( | |||
| 6 | 6 | "github.com/bnema/vev/internal/ports" | |
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | + | var errSessionMetaUnavailable = errors.New("session metadata is unavailable") | |
| 10 | + | ||
| 9 | 11 | // sessionMetaSnapshot captures the authoritative tab order while session.mu | |
| 10 | 12 | // owns every field represented on the wire. | |
| 11 | 13 | func (s *session) sessionMetaSnapshot() (ports.SessionMeta, bool) { | |
| @@ -67,7 +69,7 @@ func (ac *attachedClient) sendSessionMetaIfChanged(sess *session, expected trans | |||
| 67 | 69 | } | |
| 68 | 70 | meta, ok := sess.sessionMetaSnapshot() | |
| 69 | 71 | if !ok { | |
| 70 | - | return errors.New("session metadata is unavailable") | |
| 72 | + | return errSessionMetaUnavailable | |
| 71 | 73 | } | |
| 72 | 74 | if ac.sessionMetaSent && sameSessionMeta(ac.sessionMeta, meta) { | |
| 73 | 75 | return nil | |
| @@ -76,11 +78,8 @@ func (ac *attachedClient) sendSessionMetaIfChanged(sess *session, expected trans | |||
| 76 | 78 | if err != nil { | |
| 77 | 79 | return err | |
| 78 | 80 | } | |
| 79 | - | if expected.transport == nil || !ac.transportSnapshotCurrent(expected) { | |
| 80 | - | return errTransportReplaced | |
| 81 | - | } | |
| 82 | - | if ticket != nil && (ticket.ended.Load() || !ticket.beginTransportSend(expected)) { | |
| 83 | - | return errAttachmentTransition | |
| 81 | + | if err := ac.beginExpectedTransportSendLocked(expected, ticket); err != nil { | |
| 82 | + | return err | |
| 84 | 83 | } | |
| 85 | 84 | send := expected.transport.Send | |
| 86 | 85 | if async, ok := expected.transport.(ports.AsyncTransport); ok { | |
| @@ -6,6 +6,8 @@ import ( | |||||
| 6 | "github.com/bnema/vev/internal/ports" | 6 | "github.com/bnema/vev/internal/ports" | ||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | + | var errSessionMetaUnavailable = errors.New("session metadata is unavailable") | |||
| 10 | + | ||||
| 9 | // sessionMetaSnapshot captures the authoritative tab order while session.mu | 11 | // sessionMetaSnapshot captures the authoritative tab order while session.mu | ||
| 10 | // owns every field represented on the wire. | 12 | // owns every field represented on the wire. | ||
| 11 | func (s *session) sessionMetaSnapshot() (ports.SessionMeta, bool) { | 13 | func (s *session) sessionMetaSnapshot() (ports.SessionMeta, bool) { | ||
| @@ -67,7 +69,7 @@ func (ac *attachedClient) sendSessionMetaIfChanged(sess *session, expected trans | |||||
| 67 | } | 69 | } | ||
| 68 | meta, ok := sess.sessionMetaSnapshot() | 70 | meta, ok := sess.sessionMetaSnapshot() | ||
| 69 | if !ok { | 71 | if !ok { | ||
| 70 | - | return errors.New("session metadata is unavailable") | 72 | + | return errSessionMetaUnavailable |
| 71 | } | 73 | } | ||
| 72 | if ac.sessionMetaSent && sameSessionMeta(ac.sessionMeta, meta) { | 74 | if ac.sessionMetaSent && sameSessionMeta(ac.sessionMeta, meta) { | ||
| 73 | return nil | 75 | return nil | ||
| @@ -76,11 +78,8 @@ func (ac *attachedClient) sendSessionMetaIfChanged(sess *session, expected trans | |||||
| 76 | if err != nil { | 78 | if err != nil { | ||
| 77 | return err | 79 | return err | ||
| 78 | } | 80 | } | ||
| 79 | - | if expected.transport == nil || !ac.transportSnapshotCurrent(expected) { | 81 | + | if err := ac.beginExpectedTransportSendLocked(expected, ticket); err != nil { |
| 80 | - | return errTransportReplaced | 82 | + | return err |
| 81 | - | } | |||
| 82 | - | if ticket != nil && (ticket.ended.Load() || !ticket.beginTransportSend(expected)) { | |||
| 83 | - | return errAttachmentTransition | |||
| 84 | } | 83 | } | ||
| 85 | send := expected.transport.Send | 84 | send := expected.transport.Send | ||
| 86 | if async, ok := expected.transport.(ports.AsyncTransport); ok { | 85 | if async, ok := expected.transport.(ports.AsyncTransport); ok { | ||
M internal/usecase/daemon/session_meta_test.go +43 -4
| @@ -49,6 +49,37 @@ func TestProxiedMetadataPrecedesChangedOutput(t *testing.T) { | |||
| 49 | 49 | require.Equal(t, "renamed", changedMeta.Tabs[0].Name) | |
| 50 | 50 | } | |
| 51 | 51 | ||
| 52 | + | func TestProxiedMetadataSendsWithoutOutputBytes(t *testing.T) { | |
| 53 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 54 | + | ac.proxied = true | |
| 55 | + | ||
| 56 | + | emit := func(reset bool) { | |
| 57 | + | state := cacheBarState("steady", reset) | |
| 58 | + | state.attachment = ac | |
| 59 | + | composed := composeFrame(state, ac.pipelineCache, ac.pipelineScratch) | |
| 60 | + | ac.sendMu.Lock() | |
| 61 | + | require.True(t, d.emitFrame(sess, ac, &state, composed)) | |
| 62 | + | } | |
| 63 | + | ||
| 64 | + | emit(true) | |
| 65 | + | require.Equal(t, ports.MsgSessionMeta, (<-sends).Type) | |
| 66 | + | require.Equal(t, ports.MsgOutput, (<-sends).Type) | |
| 67 | + | ||
| 68 | + | sess.mu.Lock() | |
| 69 | + | sess.tabs[0].name = "renamed" | |
| 70 | + | sess.mu.Unlock() | |
| 71 | + | // The recomposed frame is identical, so this transaction carries metadata | |
| 72 | + | // only: a zero-byte terminal frame must still refresh the proxied snapshot. | |
| 73 | + | emit(false) | |
| 74 | + | ||
| 75 | + | metaFrame := awaitTestValue(t, sends, "a zero-byte frame did not publish changed metadata") | |
| 76 | + | require.Equal(t, ports.MsgSessionMeta, metaFrame.Type) | |
| 77 | + | meta, err := ports.UnmarshalSessionMeta(metaFrame.Payload) | |
| 78 | + | require.NoError(t, err) | |
| 79 | + | require.Equal(t, "renamed", meta.Tabs[0].Name) | |
| 80 | + | requireNoOutputFrame(t, sends) | |
| 81 | + | } | |
| 82 | + | ||
| 52 | 83 | func TestSessionMetaSnapshotIsImmutable(t *testing.T) { | |
| 53 | 84 | _, sess, _, _ := newManualSessionWithPTYs(t, newQuietPTY()) | |
| 54 | 85 | meta, ok := sess.sessionMetaSnapshot() | |
| @@ -77,26 +108,34 @@ func TestSessionMetaShadowUpdatesOnlyAfterSuccessfulSend(t *testing.T) { | |||
| 77 | 108 | ||
| 78 | 109 | func TestProxiedAttachedCommandRejectionPreservesRequestID(t *testing.T) { | |
| 79 | 110 | tests := []struct { | |
| 80 | - | name string | |
| 81 | - | request ports.CommandRequest | |
| 111 | + | name string | |
| 112 | + | request ports.CommandRequest | |
| 113 | + | wantCode uint16 | |
| 114 | + | wantText string | |
| 82 | 115 | }{ | |
| 83 | 116 | { | |
| 84 | 117 | name: "valid attached command", | |
| 85 | 118 | request: ports.CommandRequest{ | |
| 86 | 119 | Version: ports.ProtocolVersion, RequestID: 41, Attached: true, Slug: "new-tab", | |
| 87 | 120 | }, | |
| 121 | + | wantCode: ports.ErrNotScriptable, | |
| 122 | + | wantText: "attached command relay is not enabled", | |
| 88 | 123 | }, | |
| 89 | 124 | { | |
| 90 | 125 | name: "target override is rejected", | |
| 91 | 126 | request: ports.CommandRequest{ | |
| 92 | 127 | Version: ports.ProtocolVersion, RequestID: 42, Attached: true, Slug: "new-tab", TargetSession: "other", | |
| 93 | 128 | }, | |
| 129 | + | wantCode: ports.ErrNotScriptable, | |
| 130 | + | wantText: "attached command relay is not enabled", | |
| 94 | 131 | }, | |
| 95 | 132 | { | |
| 96 | 133 | name: "missing attached flag is rejected", | |
| 97 | 134 | request: ports.CommandRequest{ | |
| 98 | 135 | Version: ports.ProtocolVersion, RequestID: 43, Slug: "new-tab", | |
| 99 | 136 | }, | |
| 137 | + | wantCode: ports.ErrNotScriptable, | |
| 138 | + | wantText: "attached command relay is not enabled", | |
| 100 | 139 | }, | |
| 101 | 140 | } | |
| 102 | 141 | for _, tt := range tests { | |
| @@ -116,8 +155,8 @@ func TestProxiedAttachedCommandRejectionPreservesRequestID(t *testing.T) { | |||
| 116 | 155 | require.NoError(t, err) | |
| 117 | 156 | require.Equal(t, tt.request.RequestID, result.RequestID) | |
| 118 | 157 | require.False(t, result.OK) | |
| 119 | - | require.Equal(t, ports.ErrNotScriptable, result.Code) | |
| 120 | - | require.Equal(t, "attached command relay is not enabled", result.Text) | |
| 158 | + | require.Equal(t, tt.wantCode, result.Code) | |
| 159 | + | require.Equal(t, tt.wantText, result.Text) | |
| 121 | 160 | }) | |
| 122 | 161 | } | |
| 123 | 162 | } | |
| @@ -49,6 +49,37 @@ func TestProxiedMetadataPrecedesChangedOutput(t *testing.T) { | |||||
| 49 | require.Equal(t, "renamed", changedMeta.Tabs[0].Name) | 49 | require.Equal(t, "renamed", changedMeta.Tabs[0].Name) | ||
| 50 | } | 50 | } | ||
| 51 | 51 | ||||
| 52 | + | func TestProxiedMetadataSendsWithoutOutputBytes(t *testing.T) { | |||
| 53 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, newQuietPTY()) | |||
| 54 | + | ac.proxied = true | |||
| 55 | + | ||||
| 56 | + | emit := func(reset bool) { | |||
| 57 | + | state := cacheBarState("steady", reset) | |||
| 58 | + | state.attachment = ac | |||
| 59 | + | composed := composeFrame(state, ac.pipelineCache, ac.pipelineScratch) | |||
| 60 | + | ac.sendMu.Lock() | |||
| 61 | + | require.True(t, d.emitFrame(sess, ac, &state, composed)) | |||
| 62 | + | } | |||
| 63 | + | ||||
| 64 | + | emit(true) | |||
| 65 | + | require.Equal(t, ports.MsgSessionMeta, (<-sends).Type) | |||
| 66 | + | require.Equal(t, ports.MsgOutput, (<-sends).Type) | |||
| 67 | + | ||||
| 68 | + | sess.mu.Lock() | |||
| 69 | + | sess.tabs[0].name = "renamed" | |||
| 70 | + | sess.mu.Unlock() | |||
| 71 | + | // The recomposed frame is identical, so this transaction carries metadata | |||
| 72 | + | // only: a zero-byte terminal frame must still refresh the proxied snapshot. | |||
| 73 | + | emit(false) | |||
| 74 | + | ||||
| 75 | + | metaFrame := awaitTestValue(t, sends, "a zero-byte frame did not publish changed metadata") | |||
| 76 | + | require.Equal(t, ports.MsgSessionMeta, metaFrame.Type) | |||
| 77 | + | meta, err := ports.UnmarshalSessionMeta(metaFrame.Payload) | |||
| 78 | + | require.NoError(t, err) | |||
| 79 | + | require.Equal(t, "renamed", meta.Tabs[0].Name) | |||
| 80 | + | requireNoOutputFrame(t, sends) | |||
| 81 | + | } | |||
| 82 | + | ||||
| 52 | func TestSessionMetaSnapshotIsImmutable(t *testing.T) { | 83 | func TestSessionMetaSnapshotIsImmutable(t *testing.T) { | ||
| 53 | _, sess, _, _ := newManualSessionWithPTYs(t, newQuietPTY()) | 84 | _, sess, _, _ := newManualSessionWithPTYs(t, newQuietPTY()) | ||
| 54 | meta, ok := sess.sessionMetaSnapshot() | 85 | meta, ok := sess.sessionMetaSnapshot() | ||
| @@ -77,26 +108,34 @@ func TestSessionMetaShadowUpdatesOnlyAfterSuccessfulSend(t *testing.T) { | |||||
| 77 | 108 | ||||
| 78 | func TestProxiedAttachedCommandRejectionPreservesRequestID(t *testing.T) { | 109 | func TestProxiedAttachedCommandRejectionPreservesRequestID(t *testing.T) { | ||
| 79 | tests := []struct { | 110 | tests := []struct { | ||
| 80 | - | name string | 111 | + | name string |
| 81 | - | request ports.CommandRequest | 112 | + | request ports.CommandRequest |
| 113 | + | wantCode uint16 | |||
| 114 | + | wantText string | |||
| 82 | }{ | 115 | }{ | ||
| 83 | { | 116 | { | ||
| 84 | name: "valid attached command", | 117 | name: "valid attached command", | ||
| 85 | request: ports.CommandRequest{ | 118 | request: ports.CommandRequest{ | ||
| 86 | Version: ports.ProtocolVersion, RequestID: 41, Attached: true, Slug: "new-tab", | 119 | Version: ports.ProtocolVersion, RequestID: 41, Attached: true, Slug: "new-tab", | ||
| 87 | }, | 120 | }, | ||
| 121 | + | wantCode: ports.ErrNotScriptable, | |||
| 122 | + | wantText: "attached command relay is not enabled", | |||
| 88 | }, | 123 | }, | ||
| 89 | { | 124 | { | ||
| 90 | name: "target override is rejected", | 125 | name: "target override is rejected", | ||
| 91 | request: ports.CommandRequest{ | 126 | request: ports.CommandRequest{ | ||
| 92 | Version: ports.ProtocolVersion, RequestID: 42, Attached: true, Slug: "new-tab", TargetSession: "other", | 127 | Version: ports.ProtocolVersion, RequestID: 42, Attached: true, Slug: "new-tab", TargetSession: "other", | ||
| 93 | }, | 128 | }, | ||
| 129 | + | wantCode: ports.ErrNotScriptable, | |||
| 130 | + | wantText: "attached command relay is not enabled", | |||
| 94 | }, | 131 | }, | ||
| 95 | { | 132 | { | ||
| 96 | name: "missing attached flag is rejected", | 133 | name: "missing attached flag is rejected", | ||
| 97 | request: ports.CommandRequest{ | 134 | request: ports.CommandRequest{ | ||
| 98 | Version: ports.ProtocolVersion, RequestID: 43, Slug: "new-tab", | 135 | Version: ports.ProtocolVersion, RequestID: 43, Slug: "new-tab", | ||
| 99 | }, | 136 | }, | ||
| 137 | + | wantCode: ports.ErrNotScriptable, | |||
| 138 | + | wantText: "attached command relay is not enabled", | |||
| 100 | }, | 139 | }, | ||
| 101 | } | 140 | } | ||
| 102 | for _, tt := range tests { | 141 | for _, tt := range tests { | ||
| @@ -116,8 +155,8 @@ func TestProxiedAttachedCommandRejectionPreservesRequestID(t *testing.T) { | |||||
| 116 | require.NoError(t, err) | 155 | require.NoError(t, err) | ||
| 117 | require.Equal(t, tt.request.RequestID, result.RequestID) | 156 | require.Equal(t, tt.request.RequestID, result.RequestID) | ||
| 118 | require.False(t, result.OK) | 157 | require.False(t, result.OK) | ||
| 119 | - | require.Equal(t, ports.ErrNotScriptable, result.Code) | 158 | + | require.Equal(t, tt.wantCode, result.Code) |
| 120 | - | require.Equal(t, "attached command relay is not enabled", result.Text) | 159 | + | require.Equal(t, tt.wantText, result.Text) |
| 121 | }) | 160 | }) | ||
| 122 | } | 161 | } | ||
| 123 | } | 162 | } | ||