fix(daemon): apply follow-up review fixes
- commit
- 530da1f242a9f3ce7779870f9ecb0f79ca5e3453
- author
- brice · · 2026-08-02 11:53 UTC
- parent
- f91f9d4
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +132 -28
M internal/usecase/daemon/output_state_stream.go +2 -1
| @@ -12,7 +12,8 @@ import ( | |||
| 12 | 12 | // A reset frame is the only dependency-free output and advertises state 0 as | |
| 13 | 13 | // its base. | |
| 14 | 14 | // | |
| 15 | - | // Callers serialize access with attachedClient.sendMu. | |
| 15 | + | // Callers serialize access with attachedClient.sendMu. A prepared transaction | |
| 16 | + | // must be sent or committed as a no-op before the next transaction is prepared. | |
| 16 | 17 | type outputStateStream struct { | |
| 17 | 18 | renderer *renderer.Renderer | |
| 18 | 19 | next uint64 | |
| @@ -12,7 +12,8 @@ import ( | |||||
| 12 | // A reset frame is the only dependency-free output and advertises state 0 as | 12 | // A reset frame is the only dependency-free output and advertises state 0 as | ||
| 13 | // its base. | 13 | // its base. | ||
| 14 | // | 14 | // | ||
| 15 | - | // Callers serialize access with attachedClient.sendMu. | 15 | + | // Callers serialize access with attachedClient.sendMu. A prepared transaction |
| 16 | + | // must be sent or committed as a no-op before the next transaction is prepared. | |||
| 16 | type outputStateStream struct { | 17 | type outputStateStream struct { | ||
| 17 | renderer *renderer.Renderer | 18 | renderer *renderer.Renderer | ||
| 18 | next uint64 | 19 | next uint64 | ||
M internal/usecase/daemon/proxy_screen_state.go +23 -23
| @@ -183,15 +183,8 @@ func applyScreenUpdate(frame *renderer.Frame, update ports.ScreenUpdate) { | |||
| 183 | 183 | } | |
| 184 | 184 | ||
| 185 | 185 | func (s *proxyScreenState) recordUpdate(update ports.ScreenUpdate) { | |
| 186 | - | s.generation++ | |
| 187 | 186 | if update.Kind == ports.ScreenUpdateSnapshot { | |
| 188 | - | s.setFullRedraw() | |
| 189 | - | return | |
| 190 | - | } | |
| 191 | - | if s.damageFullRedrawSticky { | |
| 192 | - | return | |
| 193 | - | } | |
| 194 | - | if update.Scroll != nil && len(s.damage) > 0 && !(len(s.damage) == 1 && s.damage[0].Kind == renderer.DamageFullRedraw) { | |
| 187 | + | s.generation++ | |
| 195 | 188 | s.setFullRedraw() | |
| 196 | 189 | return | |
| 197 | 190 | } | |
| @@ -202,6 +195,14 @@ func (s *proxyScreenState) recordUpdate(update ports.ScreenUpdate) { | |||
| 202 | 195 | if needed == 0 { | |
| 203 | 196 | return | |
| 204 | 197 | } | |
| 198 | + | s.generation++ | |
| 199 | + | if s.damageFullRedrawSticky { | |
| 200 | + | return | |
| 201 | + | } | |
| 202 | + | if update.Scroll != nil && len(s.damage) > 0 && !(len(s.damage) == 1 && s.damage[0].Kind == renderer.DamageFullRedraw) { | |
| 203 | + | s.setFullRedraw() | |
| 204 | + | return | |
| 205 | + | } | |
| 205 | 206 | if needed > maxProxyScreenDamage-len(s.damage) { | |
| 206 | 207 | s.setFullRedraw() | |
| 207 | 208 | return | |
| @@ -346,29 +347,28 @@ func validProxyFrame(s *proxyScreenState) bool { | |||
| 346 | 347 | return s != nil && s.frame.Validate() == nil && s.scratch.Validate() == nil | |
| 347 | 348 | } | |
| 348 | 349 | ||
| 350 | + | // validProxyScreenStyle mirrors the wire validator without exporting its | |
| 351 | + | // wire-specific error API; inactive color indices are canonicalized on encode. | |
| 349 | 352 | func validProxyScreenStyle(style renderer.Style) bool { | |
| 350 | 353 | if style.Attrs&^(renderer.AttrDim|renderer.AttrUnderline|renderer.AttrBlink|renderer.AttrStrikethrough) != 0 || style.UnderlineStyle > renderer.UnderlineDashed { | |
| 351 | 354 | return false | |
| 352 | 355 | } | |
| 353 | - | if style.HasForegroundRGB { | |
| 354 | - | if style.Foreground < -1 || style.Foreground > math.MaxInt16 { | |
| 355 | - | return false | |
| 356 | - | } | |
| 357 | - | } else if style.Foreground < -1 || style.Foreground > math.MaxUint8 || style.ForegroundRGB != (renderer.RGB{}) { | |
| 356 | + | if !style.HasForegroundRGB && (style.Foreground < -1 || style.Foreground > math.MaxUint8 || style.ForegroundRGB != (renderer.RGB{})) { | |
| 358 | 357 | return false | |
| 359 | 358 | } | |
| 360 | - | if style.HasBackgroundRGB { | |
| 361 | - | if style.Background < -1 || style.Background > math.MaxInt16 { | |
| 359 | + | if !style.HasBackgroundRGB && (style.Background < -1 || style.Background > math.MaxUint8 || style.BackgroundRGB != (renderer.RGB{})) { | |
| 360 | + | return false | |
| 361 | + | } | |
| 362 | + | if style.HasUnderlineColorRGB && style.HasUnderlineColor { | |
| 363 | + | return false | |
| 364 | + | } else if style.HasUnderlineColorRGB { | |
| 365 | + | // The indexed value is inactive when RGB is selected. | |
| 366 | + | } else if style.HasUnderlineColor { | |
| 367 | + | if style.UnderlineColor < 0 || style.UnderlineColor > math.MaxUint8 || style.UnderlineColorRGB != (renderer.RGB{}) { | |
| 362 | 368 | return false | |
| 363 | 369 | } | |
| 364 | - | } else if style.Background < -1 || style.Background > math.MaxUint8 || style.BackgroundRGB != (renderer.RGB{}) { | |
| 370 | + | } else if style.UnderlineColorRGB != (renderer.RGB{}) { | |
| 365 | 371 | return false | |
| 366 | 372 | } | |
| 367 | - | if style.HasUnderlineColorRGB { | |
| 368 | - | return !style.HasUnderlineColor && style.UnderlineColor >= -1 && style.UnderlineColor <= math.MaxInt16 | |
| 369 | - | } | |
| 370 | - | if style.HasUnderlineColor { | |
| 371 | - | return style.UnderlineColor >= 0 && style.UnderlineColor <= math.MaxUint8 && style.UnderlineColorRGB == (renderer.RGB{}) | |
| 372 | - | } | |
| 373 | - | return (style.UnderlineColor == 0 || style.UnderlineColor == -1) && style.UnderlineColorRGB == (renderer.RGB{}) | |
| 373 | + | return true | |
| 374 | 374 | } | |
| @@ -183,15 +183,8 @@ func applyScreenUpdate(frame *renderer.Frame, update ports.ScreenUpdate) { | |||||
| 183 | } | 183 | } | ||
| 184 | 184 | ||||
| 185 | func (s *proxyScreenState) recordUpdate(update ports.ScreenUpdate) { | 185 | func (s *proxyScreenState) recordUpdate(update ports.ScreenUpdate) { | ||
| 186 | - | s.generation++ | |||
| 187 | if update.Kind == ports.ScreenUpdateSnapshot { | 186 | if update.Kind == ports.ScreenUpdateSnapshot { | ||
| 188 | - | s.setFullRedraw() | 187 | + | s.generation++ |
| 189 | - | return | |||
| 190 | - | } | |||
| 191 | - | if s.damageFullRedrawSticky { | |||
| 192 | - | return | |||
| 193 | - | } | |||
| 194 | - | if update.Scroll != nil && len(s.damage) > 0 && !(len(s.damage) == 1 && s.damage[0].Kind == renderer.DamageFullRedraw) { | |||
| 195 | s.setFullRedraw() | 188 | s.setFullRedraw() | ||
| 196 | return | 189 | return | ||
| 197 | } | 190 | } | ||
| @@ -202,6 +195,14 @@ func (s *proxyScreenState) recordUpdate(update ports.ScreenUpdate) { | |||||
| 202 | if needed == 0 { | 195 | if needed == 0 { | ||
| 203 | return | 196 | return | ||
| 204 | } | 197 | } | ||
| 198 | + | s.generation++ | |||
| 199 | + | if s.damageFullRedrawSticky { | |||
| 200 | + | return | |||
| 201 | + | } | |||
| 202 | + | if update.Scroll != nil && len(s.damage) > 0 && !(len(s.damage) == 1 && s.damage[0].Kind == renderer.DamageFullRedraw) { | |||
| 203 | + | s.setFullRedraw() | |||
| 204 | + | return | |||
| 205 | + | } | |||
| 205 | if needed > maxProxyScreenDamage-len(s.damage) { | 206 | if needed > maxProxyScreenDamage-len(s.damage) { | ||
| 206 | s.setFullRedraw() | 207 | s.setFullRedraw() | ||
| 207 | return | 208 | return | ||
| @@ -346,29 +347,28 @@ func validProxyFrame(s *proxyScreenState) bool { | |||||
| 346 | return s != nil && s.frame.Validate() == nil && s.scratch.Validate() == nil | 347 | return s != nil && s.frame.Validate() == nil && s.scratch.Validate() == nil | ||
| 347 | } | 348 | } | ||
| 348 | 349 | ||||
| 350 | + | // validProxyScreenStyle mirrors the wire validator without exporting its | |||
| 351 | + | // wire-specific error API; inactive color indices are canonicalized on encode. | |||
| 349 | func validProxyScreenStyle(style renderer.Style) bool { | 352 | func validProxyScreenStyle(style renderer.Style) bool { | ||
| 350 | if style.Attrs&^(renderer.AttrDim|renderer.AttrUnderline|renderer.AttrBlink|renderer.AttrStrikethrough) != 0 || style.UnderlineStyle > renderer.UnderlineDashed { | 353 | if style.Attrs&^(renderer.AttrDim|renderer.AttrUnderline|renderer.AttrBlink|renderer.AttrStrikethrough) != 0 || style.UnderlineStyle > renderer.UnderlineDashed { | ||
| 351 | return false | 354 | return false | ||
| 352 | } | 355 | } | ||
| 353 | - | if style.HasForegroundRGB { | 356 | + | if !style.HasForegroundRGB && (style.Foreground < -1 || style.Foreground > math.MaxUint8 || style.ForegroundRGB != (renderer.RGB{})) { |
| 354 | - | if style.Foreground < -1 || style.Foreground > math.MaxInt16 { | |||
| 355 | - | return false | |||
| 356 | - | } | |||
| 357 | - | } else if style.Foreground < -1 || style.Foreground > math.MaxUint8 || style.ForegroundRGB != (renderer.RGB{}) { | |||
| 358 | return false | 357 | return false | ||
| 359 | } | 358 | } | ||
| 360 | - | if style.HasBackgroundRGB { | 359 | + | if !style.HasBackgroundRGB && (style.Background < -1 || style.Background > math.MaxUint8 || style.BackgroundRGB != (renderer.RGB{})) { |
| 361 | - | if style.Background < -1 || style.Background > math.MaxInt16 { | 360 | + | return false |
| 361 | + | } | |||
| 362 | + | if style.HasUnderlineColorRGB && style.HasUnderlineColor { | |||
| 363 | + | return false | |||
| 364 | + | } else if style.HasUnderlineColorRGB { | |||
| 365 | + | // The indexed value is inactive when RGB is selected. | |||
| 366 | + | } else if style.HasUnderlineColor { | |||
| 367 | + | if style.UnderlineColor < 0 || style.UnderlineColor > math.MaxUint8 || style.UnderlineColorRGB != (renderer.RGB{}) { | |||
| 362 | return false | 368 | return false | ||
| 363 | } | 369 | } | ||
| 364 | - | } else if style.Background < -1 || style.Background > math.MaxUint8 || style.BackgroundRGB != (renderer.RGB{}) { | 370 | + | } else if style.UnderlineColorRGB != (renderer.RGB{}) { |
| 365 | return false | 371 | return false | ||
| 366 | } | 372 | } | ||
| 367 | - | if style.HasUnderlineColorRGB { | 373 | + | return true |
| 368 | - | return !style.HasUnderlineColor && style.UnderlineColor >= -1 && style.UnderlineColor <= math.MaxInt16 | |||
| 369 | - | } | |||
| 370 | - | if style.HasUnderlineColor { | |||
| 371 | - | return style.UnderlineColor >= 0 && style.UnderlineColor <= math.MaxUint8 && style.UnderlineColorRGB == (renderer.RGB{}) | |||
| 372 | - | } | |||
| 373 | - | return (style.UnderlineColor == 0 || style.UnderlineColor == -1) && style.UnderlineColorRGB == (renderer.RGB{}) | |||
| 374 | } | 374 | } | ||
M internal/usecase/daemon/proxy_screen_state_test.go +67 -0
| @@ -329,6 +329,40 @@ func TestProxyScreenStateCaptureFallsBackAfterRepeatedScroll(t *testing.T) { | |||
| 329 | 329 | } | |
| 330 | 330 | } | |
| 331 | 331 | ||
| 332 | + | func TestProxyScreenStateCursorOnlyPreservesCapturedDamageGeneration(t *testing.T) { | |
| 333 | + | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | |
| 334 | + | if err := s.Apply(screenSnapshot(2, 2, " ", " ")); err != nil { | |
| 335 | + | t.Fatal(err) | |
| 336 | + | } | |
| 337 | + | snapshotDamage := s.CaptureDamage() | |
| 338 | + | applyProxyDelta(t, s, ports.ScreenUpdate{ | |
| 339 | + | Size: domain.Size{Cols: 2, Rows: 2}, | |
| 340 | + | Cursor: ports.ScreenCursor{Row: 1, Col: 1, Visible: true}, | |
| 341 | + | }) | |
| 342 | + | if s.generation != snapshotDamage.Generation { | |
| 343 | + | t.Fatalf("cursor-only update changed damage generation from %d to %d", snapshotDamage.Generation, s.generation) | |
| 344 | + | } | |
| 345 | + | if !s.AcknowledgeDamage(snapshotDamage.Generation) || len(s.damage) != 0 || s.damageFullRedrawSticky { | |
| 346 | + | t.Fatalf("cursor-only interleaving invalidated captured damage: generation=%d damage=%+v sticky=%v", s.generation, s.damage, s.damageFullRedrawSticky) | |
| 347 | + | } | |
| 348 | + | ||
| 349 | + | applyProxyDelta(t, s, ports.ScreenUpdate{ | |
| 350 | + | Size: domain.Size{Cols: 2, Rows: 2}, | |
| 351 | + | Spans: []ports.ScreenSpan{{Y: 0, Cells: cells("x")}}, | |
| 352 | + | }) | |
| 353 | + | actualDamage := s.CaptureDamage() | |
| 354 | + | if actualDamage.Generation <= snapshotDamage.Generation { | |
| 355 | + | t.Fatalf("actual damage did not advance generation: snapshot=%d actual=%d", snapshotDamage.Generation, actualDamage.Generation) | |
| 356 | + | } | |
| 357 | + | applyProxyDelta(t, s, ports.ScreenUpdate{ | |
| 358 | + | Size: domain.Size{Cols: 2, Rows: 2}, | |
| 359 | + | Cursor: ports.ScreenCursor{Row: 0, Col: 1, Visible: true}, | |
| 360 | + | }) | |
| 361 | + | if s.generation != actualDamage.Generation || !s.AcknowledgeDamage(actualDamage.Generation) || len(s.damage) != 0 || s.damageFullRedrawSticky { | |
| 362 | + | t.Fatalf("cursor-only update invalidated actual damage: generation=%d want=%d damage=%+v sticky=%v", s.generation, actualDamage.Generation, s.damage, s.damageFullRedrawSticky) | |
| 363 | + | } | |
| 364 | + | } | |
| 365 | + | ||
| 332 | 366 | func TestProxyScreenStateCursorOnlyAndDamageBound(t *testing.T) { | |
| 333 | 367 | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | |
| 334 | 368 | if err := s.Apply(screenSnapshot(2, 2, " ", " ")); err != nil { | |
| @@ -360,6 +394,39 @@ func TestProxyScreenStateCursorOnlyAndDamageBound(t *testing.T) { | |||
| 360 | 394 | } | |
| 361 | 395 | } | |
| 362 | 396 | ||
| 397 | + | func TestProxyScreenStateStyleValidationMatchesWireSemantics(t *testing.T) { | |
| 398 | + | styles := []struct { | |
| 399 | + | name string | |
| 400 | + | style renderer.Style | |
| 401 | + | }{ | |
| 402 | + | {name: "indexed foreground", style: renderer.Style{Foreground: 255, Background: -1}}, | |
| 403 | + | {name: "indexed foreground out of range", style: renderer.Style{Foreground: 256, Background: -1}}, | |
| 404 | + | {name: "foreground RGB ignores index", style: renderer.Style{Foreground: 1 << 20, Background: -1, HasForegroundRGB: true, ForegroundRGB: renderer.RGB{R: 1}}}, | |
| 405 | + | {name: "background RGB ignores index", style: renderer.Style{Foreground: -1, Background: 1 << 20, HasBackgroundRGB: true, BackgroundRGB: renderer.RGB{G: 2}}}, | |
| 406 | + | {name: "indexed underline", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColor: true, UnderlineColor: 255}}, | |
| 407 | + | {name: "indexed underline with RGB", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColor: true, UnderlineColor: 1, UnderlineColorRGB: renderer.RGB{B: 3}}}, | |
| 408 | + | {name: "underline RGB ignores index", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColorRGB: true, UnderlineColor: 1 << 20, UnderlineColorRGB: renderer.RGB{R: 4}}}, | |
| 409 | + | {name: "both underline modes", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColor: true, HasUnderlineColorRGB: true}}, | |
| 410 | + | {name: "unset underline ignores index", style: renderer.Style{Foreground: -1, Background: -1, UnderlineColor: 1 << 20}}, | |
| 411 | + | } | |
| 412 | + | for _, tt := range styles { | |
| 413 | + | t.Run(tt.name, func(t *testing.T) { | |
| 414 | + | update := ports.ScreenUpdate{ | |
| 415 | + | NewStateNum: 1, | |
| 416 | + | Kind: ports.ScreenUpdateSnapshot, | |
| 417 | + | Size: domain.Size{Cols: 1, Rows: 1}, | |
| 418 | + | Cursor: ports.ScreenCursor{Visible: true}, | |
| 419 | + | Spans: []ports.ScreenSpan{{Cells: []renderer.Cell{{Rune: 'x', Style: tt.style}}}}, | |
| 420 | + | } | |
| 421 | + | _, wireErr := ports.MarshalScreenUpdate(update) | |
| 422 | + | stateErr := newProxyScreenState(update.Size).Apply(update) | |
| 423 | + | if (wireErr == nil) != (stateErr == nil) { | |
| 424 | + | t.Fatalf("daemon/wire style acceptance differs: wire=%v daemon=%v", wireErr, stateErr) | |
| 425 | + | } | |
| 426 | + | }) | |
| 427 | + | } | |
| 428 | + | } | |
| 429 | + | ||
| 363 | 430 | // BenchmarkProxyScreenStateApply is a component diagnostic for applying an | |
| 364 | 431 | // already-decoded update; it is not a pipeline comparison with ANSI apply. | |
| 365 | 432 | func BenchmarkProxyScreenStateApply(b *testing.B) { | |
| @@ -329,6 +329,40 @@ func TestProxyScreenStateCaptureFallsBackAfterRepeatedScroll(t *testing.T) { | |||||
| 329 | } | 329 | } | ||
| 330 | } | 330 | } | ||
| 331 | 331 | ||||
| 332 | + | func TestProxyScreenStateCursorOnlyPreservesCapturedDamageGeneration(t *testing.T) { | |||
| 333 | + | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | |||
| 334 | + | if err := s.Apply(screenSnapshot(2, 2, " ", " ")); err != nil { | |||
| 335 | + | t.Fatal(err) | |||
| 336 | + | } | |||
| 337 | + | snapshotDamage := s.CaptureDamage() | |||
| 338 | + | applyProxyDelta(t, s, ports.ScreenUpdate{ | |||
| 339 | + | Size: domain.Size{Cols: 2, Rows: 2}, | |||
| 340 | + | Cursor: ports.ScreenCursor{Row: 1, Col: 1, Visible: true}, | |||
| 341 | + | }) | |||
| 342 | + | if s.generation != snapshotDamage.Generation { | |||
| 343 | + | t.Fatalf("cursor-only update changed damage generation from %d to %d", snapshotDamage.Generation, s.generation) | |||
| 344 | + | } | |||
| 345 | + | if !s.AcknowledgeDamage(snapshotDamage.Generation) || len(s.damage) != 0 || s.damageFullRedrawSticky { | |||
| 346 | + | t.Fatalf("cursor-only interleaving invalidated captured damage: generation=%d damage=%+v sticky=%v", s.generation, s.damage, s.damageFullRedrawSticky) | |||
| 347 | + | } | |||
| 348 | + | ||||
| 349 | + | applyProxyDelta(t, s, ports.ScreenUpdate{ | |||
| 350 | + | Size: domain.Size{Cols: 2, Rows: 2}, | |||
| 351 | + | Spans: []ports.ScreenSpan{{Y: 0, Cells: cells("x")}}, | |||
| 352 | + | }) | |||
| 353 | + | actualDamage := s.CaptureDamage() | |||
| 354 | + | if actualDamage.Generation <= snapshotDamage.Generation { | |||
| 355 | + | t.Fatalf("actual damage did not advance generation: snapshot=%d actual=%d", snapshotDamage.Generation, actualDamage.Generation) | |||
| 356 | + | } | |||
| 357 | + | applyProxyDelta(t, s, ports.ScreenUpdate{ | |||
| 358 | + | Size: domain.Size{Cols: 2, Rows: 2}, | |||
| 359 | + | Cursor: ports.ScreenCursor{Row: 0, Col: 1, Visible: true}, | |||
| 360 | + | }) | |||
| 361 | + | if s.generation != actualDamage.Generation || !s.AcknowledgeDamage(actualDamage.Generation) || len(s.damage) != 0 || s.damageFullRedrawSticky { | |||
| 362 | + | t.Fatalf("cursor-only update invalidated actual damage: generation=%d want=%d damage=%+v sticky=%v", s.generation, actualDamage.Generation, s.damage, s.damageFullRedrawSticky) | |||
| 363 | + | } | |||
| 364 | + | } | |||
| 365 | + | ||||
| 332 | func TestProxyScreenStateCursorOnlyAndDamageBound(t *testing.T) { | 366 | func TestProxyScreenStateCursorOnlyAndDamageBound(t *testing.T) { | ||
| 333 | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | 367 | s := newProxyScreenState(domain.Size{Cols: 2, Rows: 2}) | ||
| 334 | if err := s.Apply(screenSnapshot(2, 2, " ", " ")); err != nil { | 368 | if err := s.Apply(screenSnapshot(2, 2, " ", " ")); err != nil { | ||
| @@ -360,6 +394,39 @@ func TestProxyScreenStateCursorOnlyAndDamageBound(t *testing.T) { | |||||
| 360 | } | 394 | } | ||
| 361 | } | 395 | } | ||
| 362 | 396 | ||||
| 397 | + | func TestProxyScreenStateStyleValidationMatchesWireSemantics(t *testing.T) { | |||
| 398 | + | styles := []struct { | |||
| 399 | + | name string | |||
| 400 | + | style renderer.Style | |||
| 401 | + | }{ | |||
| 402 | + | {name: "indexed foreground", style: renderer.Style{Foreground: 255, Background: -1}}, | |||
| 403 | + | {name: "indexed foreground out of range", style: renderer.Style{Foreground: 256, Background: -1}}, | |||
| 404 | + | {name: "foreground RGB ignores index", style: renderer.Style{Foreground: 1 << 20, Background: -1, HasForegroundRGB: true, ForegroundRGB: renderer.RGB{R: 1}}}, | |||
| 405 | + | {name: "background RGB ignores index", style: renderer.Style{Foreground: -1, Background: 1 << 20, HasBackgroundRGB: true, BackgroundRGB: renderer.RGB{G: 2}}}, | |||
| 406 | + | {name: "indexed underline", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColor: true, UnderlineColor: 255}}, | |||
| 407 | + | {name: "indexed underline with RGB", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColor: true, UnderlineColor: 1, UnderlineColorRGB: renderer.RGB{B: 3}}}, | |||
| 408 | + | {name: "underline RGB ignores index", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColorRGB: true, UnderlineColor: 1 << 20, UnderlineColorRGB: renderer.RGB{R: 4}}}, | |||
| 409 | + | {name: "both underline modes", style: renderer.Style{Foreground: -1, Background: -1, HasUnderlineColor: true, HasUnderlineColorRGB: true}}, | |||
| 410 | + | {name: "unset underline ignores index", style: renderer.Style{Foreground: -1, Background: -1, UnderlineColor: 1 << 20}}, | |||
| 411 | + | } | |||
| 412 | + | for _, tt := range styles { | |||
| 413 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 414 | + | update := ports.ScreenUpdate{ | |||
| 415 | + | NewStateNum: 1, | |||
| 416 | + | Kind: ports.ScreenUpdateSnapshot, | |||
| 417 | + | Size: domain.Size{Cols: 1, Rows: 1}, | |||
| 418 | + | Cursor: ports.ScreenCursor{Visible: true}, | |||
| 419 | + | Spans: []ports.ScreenSpan{{Cells: []renderer.Cell{{Rune: 'x', Style: tt.style}}}}, | |||
| 420 | + | } | |||
| 421 | + | _, wireErr := ports.MarshalScreenUpdate(update) | |||
| 422 | + | stateErr := newProxyScreenState(update.Size).Apply(update) | |||
| 423 | + | if (wireErr == nil) != (stateErr == nil) { | |||
| 424 | + | t.Fatalf("daemon/wire style acceptance differs: wire=%v daemon=%v", wireErr, stateErr) | |||
| 425 | + | } | |||
| 426 | + | }) | |||
| 427 | + | } | |||
| 428 | + | } | |||
| 429 | + | ||||
| 363 | // BenchmarkProxyScreenStateApply is a component diagnostic for applying an | 430 | // BenchmarkProxyScreenStateApply is a component diagnostic for applying an | ||
| 364 | // already-decoded update; it is not a pipeline comparison with ANSI apply. | 431 | // already-decoded update; it is not a pipeline comparison with ANSI apply. | ||
| 365 | func BenchmarkProxyScreenStateApply(b *testing.B) { | 432 | func BenchmarkProxyScreenStateApply(b *testing.B) { | ||
M internal/usecase/daemon/screen_output_stream.go +7 -1
| @@ -191,10 +191,16 @@ func (p *preparedStructuredOutput) send(sender func(ports.Frame) error) error { | |||
| 191 | 191 | // A failed send is already attempted, so it cannot accidentally commit after | |
| 192 | 192 | // the failure and thereby destroy the required snapshot retry. | |
| 193 | 193 | func (p *preparedStructuredOutput) commitNoSend() { | |
| 194 | - | if p == nil || p.attempted || len(p.data) != 0 { | |
| 194 | + | if p == nil || p.attempted { | |
| 195 | 195 | return | |
| 196 | 196 | } | |
| 197 | 197 | p.attempted = true | |
| 198 | + | if len(p.data) != 0 { | |
| 199 | + | if p.stream != nil { | |
| 200 | + | p.stream.forceSnapshot = true | |
| 201 | + | } | |
| 202 | + | return | |
| 203 | + | } | |
| 198 | 204 | p.commit() | |
| 199 | 205 | } | |
| 200 | 206 | ||
| @@ -191,10 +191,16 @@ func (p *preparedStructuredOutput) send(sender func(ports.Frame) error) error { | |||||
| 191 | // A failed send is already attempted, so it cannot accidentally commit after | 191 | // A failed send is already attempted, so it cannot accidentally commit after | ||
| 192 | // the failure and thereby destroy the required snapshot retry. | 192 | // the failure and thereby destroy the required snapshot retry. | ||
| 193 | func (p *preparedStructuredOutput) commitNoSend() { | 193 | func (p *preparedStructuredOutput) commitNoSend() { | ||
| 194 | - | if p == nil || p.attempted || len(p.data) != 0 { | 194 | + | if p == nil || p.attempted { |
| 195 | return | 195 | return | ||
| 196 | } | 196 | } | ||
| 197 | p.attempted = true | 197 | p.attempted = true | ||
| 198 | + | if len(p.data) != 0 { | |||
| 199 | + | if p.stream != nil { | |||
| 200 | + | p.stream.forceSnapshot = true | |||
| 201 | + | } | |||
| 202 | + | return | |||
| 203 | + | } | |||
| 198 | p.commit() | 204 | p.commit() | ||
| 199 | } | 205 | } | ||
| 200 | 206 | ||||
M internal/usecase/daemon/screen_output_stream_test.go +32 -1
| @@ -110,7 +110,7 @@ func TestStructuredOutputStreamDeltaCommitReusesShadowStorage(t *testing.T) { | |||
| 110 | 110 | require.NoError(t, err) | |
| 111 | 111 | require.NoError(t, delta.send(func(ports.Frame) error { return nil })) | |
| 112 | 112 | ||
| 113 | - | require.Equal(t, shadowStart, &stream.shadow.Cells[0]) | |
| 113 | + | require.Same(t, shadowStart, &stream.shadow.Cells[0]) | |
| 114 | 114 | require.Equal(t, shadowCap, cap(stream.shadow.Cells)) | |
| 115 | 115 | require.Equal(t, changed.At(1, 0), stream.shadow.At(1, 0)) | |
| 116 | 116 | require.Equal(t, uint64(2), state.next) | |
| @@ -184,6 +184,36 @@ func TestStructuredOutputStreamFailedSendRetainsTransactionAndRetriesSnapshot(t | |||
| 184 | 184 | })) | |
| 185 | 185 | } | |
| 186 | 186 | ||
| 187 | + | func TestStructuredOutputStreamNonNoopCommitForcesSnapshotRetry(t *testing.T) { | |
| 188 | + | state := newOutputStateStream() | |
| 189 | + | stream := newStructuredOutputStream(state) | |
| 190 | + | cursor := cursorOut{valid: true} | |
| 191 | + | frame := structuredTestFrame("abc") | |
| 192 | + | ||
| 193 | + | first, err := stream.prepare(frame, nil, cursor, true, 0) | |
| 194 | + | require.NoError(t, err) | |
| 195 | + | require.NoError(t, first.send(func(ports.Frame) error { return nil })) | |
| 196 | + | before := stream.shadow.Clone() | |
| 197 | + | ||
| 198 | + | changed := frame.Clone() | |
| 199 | + | changed.Set(0, 0, renderer.Cell{Rune: 'x', Style: renderer.DefaultStyle()}) | |
| 200 | + | pending, err := stream.prepare(changed, nil, cursor, false, 0) | |
| 201 | + | require.NoError(t, err) | |
| 202 | + | require.NotEmpty(t, pending.data) | |
| 203 | + | pending.commitNoSend() | |
| 204 | + | require.Equal(t, before, stream.shadow, "non-noop commit must not publish its candidate") | |
| 205 | + | require.Equal(t, uint64(1), state.next) | |
| 206 | + | require.True(t, stream.forceSnapshot) | |
| 207 | + | ||
| 208 | + | retry, err := stream.prepare(changed, nil, cursor, false, 0) | |
| 209 | + | require.NoError(t, err) | |
| 210 | + | retryUpdate := decodeStructured(t, retry.data) | |
| 211 | + | require.Equal(t, ports.ScreenUpdateSnapshot, retryUpdate.Kind) | |
| 212 | + | require.Zero(t, retryUpdate.BaseStateNum) | |
| 213 | + | require.NoError(t, retry.send(func(ports.Frame) error { return nil })) | |
| 214 | + | require.Equal(t, uint64(2), state.next) | |
| 215 | + | } | |
| 216 | + | ||
| 187 | 217 | func TestStructuredOutputStreamComparesCursorValidityAndHidden(t *testing.T) { | |
| 188 | 218 | state := newOutputStateStream() | |
| 189 | 219 | stream := newStructuredOutputStream(state) | |
| @@ -223,6 +253,7 @@ func TestStructuredOutputStreamRejectsInvalidCursorAndGeometry(t *testing.T) { | |||
| 223 | 253 | }{ | |
| 224 | 254 | {name: "cursor row outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, row: 1}}, | |
| 225 | 255 | {name: "cursor col outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, col: 1}}, | |
| 256 | + | {name: "cursor style out of range", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, hasStyle: true, style: 7}}, | |
| 226 | 257 | {name: "screen cell cap", frame: renderer.NewFrame(513, 512), cursor: cursorOut{valid: true}}, | |
| 227 | 258 | {name: "uint16 dimension cap", frame: renderer.NewFrame(math.MaxUint16+1, 1), cursor: cursorOut{valid: true}}, | |
| 228 | 259 | } | |
| @@ -110,7 +110,7 @@ func TestStructuredOutputStreamDeltaCommitReusesShadowStorage(t *testing.T) { | |||||
| 110 | require.NoError(t, err) | 110 | require.NoError(t, err) | ||
| 111 | require.NoError(t, delta.send(func(ports.Frame) error { return nil })) | 111 | require.NoError(t, delta.send(func(ports.Frame) error { return nil })) | ||
| 112 | 112 | ||||
| 113 | - | require.Equal(t, shadowStart, &stream.shadow.Cells[0]) | 113 | + | require.Same(t, shadowStart, &stream.shadow.Cells[0]) |
| 114 | require.Equal(t, shadowCap, cap(stream.shadow.Cells)) | 114 | require.Equal(t, shadowCap, cap(stream.shadow.Cells)) | ||
| 115 | require.Equal(t, changed.At(1, 0), stream.shadow.At(1, 0)) | 115 | require.Equal(t, changed.At(1, 0), stream.shadow.At(1, 0)) | ||
| 116 | require.Equal(t, uint64(2), state.next) | 116 | require.Equal(t, uint64(2), state.next) | ||
| @@ -184,6 +184,36 @@ func TestStructuredOutputStreamFailedSendRetainsTransactionAndRetriesSnapshot(t | |||||
| 184 | })) | 184 | })) | ||
| 185 | } | 185 | } | ||
| 186 | 186 | ||||
| 187 | + | func TestStructuredOutputStreamNonNoopCommitForcesSnapshotRetry(t *testing.T) { | |||
| 188 | + | state := newOutputStateStream() | |||
| 189 | + | stream := newStructuredOutputStream(state) | |||
| 190 | + | cursor := cursorOut{valid: true} | |||
| 191 | + | frame := structuredTestFrame("abc") | |||
| 192 | + | ||||
| 193 | + | first, err := stream.prepare(frame, nil, cursor, true, 0) | |||
| 194 | + | require.NoError(t, err) | |||
| 195 | + | require.NoError(t, first.send(func(ports.Frame) error { return nil })) | |||
| 196 | + | before := stream.shadow.Clone() | |||
| 197 | + | ||||
| 198 | + | changed := frame.Clone() | |||
| 199 | + | changed.Set(0, 0, renderer.Cell{Rune: 'x', Style: renderer.DefaultStyle()}) | |||
| 200 | + | pending, err := stream.prepare(changed, nil, cursor, false, 0) | |||
| 201 | + | require.NoError(t, err) | |||
| 202 | + | require.NotEmpty(t, pending.data) | |||
| 203 | + | pending.commitNoSend() | |||
| 204 | + | require.Equal(t, before, stream.shadow, "non-noop commit must not publish its candidate") | |||
| 205 | + | require.Equal(t, uint64(1), state.next) | |||
| 206 | + | require.True(t, stream.forceSnapshot) | |||
| 207 | + | ||||
| 208 | + | retry, err := stream.prepare(changed, nil, cursor, false, 0) | |||
| 209 | + | require.NoError(t, err) | |||
| 210 | + | retryUpdate := decodeStructured(t, retry.data) | |||
| 211 | + | require.Equal(t, ports.ScreenUpdateSnapshot, retryUpdate.Kind) | |||
| 212 | + | require.Zero(t, retryUpdate.BaseStateNum) | |||
| 213 | + | require.NoError(t, retry.send(func(ports.Frame) error { return nil })) | |||
| 214 | + | require.Equal(t, uint64(2), state.next) | |||
| 215 | + | } | |||
| 216 | + | ||||
| 187 | func TestStructuredOutputStreamComparesCursorValidityAndHidden(t *testing.T) { | 217 | func TestStructuredOutputStreamComparesCursorValidityAndHidden(t *testing.T) { | ||
| 188 | state := newOutputStateStream() | 218 | state := newOutputStateStream() | ||
| 189 | stream := newStructuredOutputStream(state) | 219 | stream := newStructuredOutputStream(state) | ||
| @@ -223,6 +253,7 @@ func TestStructuredOutputStreamRejectsInvalidCursorAndGeometry(t *testing.T) { | |||||
| 223 | }{ | 253 | }{ | ||
| 224 | {name: "cursor row outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, row: 1}}, | 254 | {name: "cursor row outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, row: 1}}, | ||
| 225 | {name: "cursor col outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, col: 1}}, | 255 | {name: "cursor col outside frame", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, col: 1}}, | ||
| 256 | + | {name: "cursor style out of range", frame: structuredTestFrame("a"), cursor: cursorOut{valid: true, hasStyle: true, style: 7}}, | |||
| 226 | {name: "screen cell cap", frame: renderer.NewFrame(513, 512), cursor: cursorOut{valid: true}}, | 257 | {name: "screen cell cap", frame: renderer.NewFrame(513, 512), cursor: cursorOut{valid: true}}, | ||
| 227 | {name: "uint16 dimension cap", frame: renderer.NewFrame(math.MaxUint16+1, 1), cursor: cursorOut{valid: true}}, | 258 | {name: "uint16 dimension cap", frame: renderer.NewFrame(math.MaxUint16+1, 1), cursor: cursorOut{valid: true}}, | ||
| 228 | } | 259 | } | ||
M internal/usecase/daemon/session_meta_test.go +1 -2
| @@ -4,7 +4,6 @@ import ( | |||
| 4 | 4 | "errors" | |
| 5 | 5 | "io" | |
| 6 | 6 | "testing" | |
| 7 | - | "time" | |
| 8 | 7 | ||
| 9 | 8 | "github.com/bnema/vev/internal/ports" | |
| 10 | 9 | "github.com/stretchr/testify/require" | |
| @@ -87,7 +86,7 @@ func TestProxiedMetadataSendsWithoutOutputBytes(t *testing.T) { | |||
| 87 | 86 | select { | |
| 88 | 87 | case frame := <-sends: | |
| 89 | 88 | t.Fatalf("unexpected frame after metadata-only paint: %v", frame.Type) | |
| 90 | - | case <-time.After(50 * time.Millisecond): | |
| 89 | + | default: | |
| 91 | 90 | } | |
| 92 | 91 | } | |
| 93 | 92 | ||
| @@ -4,7 +4,6 @@ import ( | |||||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "io" | 5 | "io" | ||
| 6 | "testing" | 6 | "testing" | ||
| 7 | - | "time" | |||
| 8 | 7 | ||||
| 9 | "github.com/bnema/vev/internal/ports" | 8 | "github.com/bnema/vev/internal/ports" | ||
| 10 | "github.com/stretchr/testify/require" | 9 | "github.com/stretchr/testify/require" | ||
| @@ -87,7 +86,7 @@ func TestProxiedMetadataSendsWithoutOutputBytes(t *testing.T) { | |||||
| 87 | select { | 86 | select { | ||
| 88 | case frame := <-sends: | 87 | case frame := <-sends: | ||
| 89 | t.Fatalf("unexpected frame after metadata-only paint: %v", frame.Type) | 88 | t.Fatalf("unexpected frame after metadata-only paint: %v", frame.Type) | ||
| 90 | - | case <-time.After(50 * time.Millisecond): | 89 | + | default: |
| 91 | } | 90 | } | ||
| 92 | } | 91 | } | ||
| 93 | 92 | ||||