fix(client): flush ambiguous escape input
- commit
- 6e4ea878acad04fb39c458cc1067bcfde41c4e67
- author
- brice · · 2026-07-18 07:07 UTC
- parent
- d885d0f
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +168 -39
M internal/usecase/client/client.go +110 -35
| @@ -910,11 +910,11 @@ func runSender(ctx context.Context, cancel context.CancelFunc, transport ports.T | |||
| 910 | 910 | // cancels the loop. | |
| 911 | 911 | // | |
| 912 | 912 | // Known MVP limitation: a bare io.Reader cannot be unblocked from outside, | |
| 913 | - | // so on shutdown initiated elsewhere (daemon detach, transport loss) this | |
| 914 | - | // goroutine stays parked in Read until the next byte arrives or the process | |
| 915 | - | // exits. That is harmless here — Run has already returned and restored the | |
| 916 | - | // terminal — and matches the standard pattern for stdin pumps; a | |
| 917 | - | // closable stdin duplicate could lift it later if ever needed. | |
| 913 | + | // so on shutdown initiated elsewhere (daemon detach, transport loss) the | |
| 914 | + | // reader goroutine stays parked in Read until the next byte arrives or the | |
| 915 | + | // process exits. That is harmless here — Run has already returned and | |
| 916 | + | // restored the terminal — and matches the standard pattern for stdin pumps; | |
| 917 | + | // a closable stdin duplicate could lift it later if ever needed. | |
| 918 | 918 | // stdinPump parses terminal color reports into generation-tagged events while | |
| 919 | 919 | // preserving all ordinary terminal input byte-for-byte. It never publishes a | |
| 920 | 920 | // Theme and never writes terminal output; attachAttempt owns both operations. | |
| @@ -978,42 +978,117 @@ func (p *stdinPump) run() { | |||
| 978 | 978 | } | |
| 979 | 979 | sink = ci.Scan | |
| 980 | 980 | } | |
| 981 | - | for { | |
| 982 | - | n, rerr := p.in.Read(buf) | |
| 983 | - | // All scanner callbacks from this Read must retain this one generation. | |
| 984 | - | // A scheme notification can start its replacement before a later byte in | |
| 985 | - | // the same read is scanned; reloading there would misclassify an old | |
| 986 | - | // completion marker as the replacement's drain response. | |
| 987 | - | readGeneration := paletteGenerationID(p.activeGeneration.Load()) | |
| 988 | - | if n > 0 { | |
| 989 | - | scanner.Scan(buf[:n], func(kind int, rgb renderer.RGB) { | |
| 990 | - | kindEvent := paletteEventForeground | |
| 991 | - | if kind == 11 { | |
| 992 | - | kindEvent = paletteEventBackground | |
| 993 | - | } | |
| 994 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: kindEvent, rgb: rgb}) | |
| 995 | - | }, func(slot int, rgb renderer.RGB) { | |
| 996 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventPalette, slot: uint8(slot), rgb: rgb}) | |
| 997 | - | }, func(light bool) { | |
| 998 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventScheme, light: light}) | |
| 999 | - | }, func(data []byte) { | |
| 1000 | - | markers.scan(data, sink, func() { | |
| 1001 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventMarker}) | |
| 1002 | - | }) | |
| 1003 | - | }) | |
| 1004 | - | if !sendOK.Load() { | |
| 1005 | - | return | |
| 981 | + | type readResult struct { | |
| 982 | + | data []byte | |
| 983 | + | err error | |
| 984 | + | } | |
| 985 | + | reads := make(chan readResult, 1) | |
| 986 | + | go func() { | |
| 987 | + | defer close(reads) | |
| 988 | + | for { | |
| 989 | + | n, err := p.in.Read(buf) | |
| 990 | + | result := readResult{err: err} | |
| 991 | + | if n > 0 { | |
| 992 | + | result.data = append([]byte(nil), buf[:n]...) | |
| 1006 | 993 | } | |
| 1007 | - | } | |
| 1008 | - | if rerr != nil { | |
| 1009 | - | markers.flush(sink) | |
| 1010 | 994 | select { | |
| 1011 | - | case p.out <- ports.Frame{Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}: | |
| 995 | + | case reads <- result: | |
| 1012 | 996 | case <-p.ctx.Done(): | |
| 997 | + | return | |
| 1013 | 998 | } | |
| 1014 | - | p.cancel() | |
| 999 | + | if err != nil { | |
| 1000 | + | return | |
| 1001 | + | } | |
| 1002 | + | } | |
| 1003 | + | }() | |
| 1004 | + | ||
| 1005 | + | var markerTimer ports.Timer | |
| 1006 | + | var markerTimerC <-chan time.Time | |
| 1007 | + | flushMarkerPrefix := func() { | |
| 1008 | + | markers.flush(sink) | |
| 1009 | + | markerTimer = nil | |
| 1010 | + | markerTimerC = nil | |
| 1011 | + | } | |
| 1012 | + | // Disarm before processing a subsequent read. A deadline already ready at | |
| 1013 | + | // that boundary wins, so bytes cannot be retroactively consumed as a | |
| 1014 | + | // marker after they were due to be forwarded as ordinary input. | |
| 1015 | + | disarmMarkerDeadline := func() { | |
| 1016 | + | if markerTimer == nil { | |
| 1017 | + | return | |
| 1018 | + | } | |
| 1019 | + | select { | |
| 1020 | + | case <-markerTimerC: | |
| 1021 | + | flushMarkerPrefix() | |
| 1022 | + | return | |
| 1023 | + | default: | |
| 1024 | + | } | |
| 1025 | + | if !markerTimer.Stop() { | |
| 1026 | + | // Stop reports false only after expiry or a prior stop. This timer is | |
| 1027 | + | // owned only here, so expiry wins even if delivery to C races this | |
| 1028 | + | // select; do not let a late value consume a newly arrived suffix. | |
| 1029 | + | flushMarkerPrefix() | |
| 1015 | 1030 | return | |
| 1016 | 1031 | } | |
| 1032 | + | markerTimer = nil | |
| 1033 | + | markerTimerC = nil | |
| 1034 | + | } | |
| 1035 | + | armMarkerDeadline := func() { | |
| 1036 | + | disarmMarkerDeadline() | |
| 1037 | + | markerTimer = p.clock.NewTimer(paletteMarkerAmbiguityDeadline) | |
| 1038 | + | markerTimerC = markerTimer.C() | |
| 1039 | + | } | |
| 1040 | + | defer disarmMarkerDeadline() | |
| 1041 | + | ||
| 1042 | + | for { | |
| 1043 | + | select { | |
| 1044 | + | case <-p.ctx.Done(): | |
| 1045 | + | return | |
| 1046 | + | case <-markerTimerC: | |
| 1047 | + | flushMarkerPrefix() | |
| 1048 | + | case result, ok := <-reads: | |
| 1049 | + | if !ok { | |
| 1050 | + | return | |
| 1051 | + | } | |
| 1052 | + | disarmMarkerDeadline() | |
| 1053 | + | // All scanner callbacks from this Read must retain this one generation. | |
| 1054 | + | // A scheme notification can start its replacement before a later byte in | |
| 1055 | + | // the same read is scanned; reloading there would misclassify an old | |
| 1056 | + | // completion marker as the replacement's drain response. | |
| 1057 | + | readGeneration := paletteGenerationID(p.activeGeneration.Load()) | |
| 1058 | + | if len(result.data) > 0 { | |
| 1059 | + | scanner.Scan(result.data, func(kind int, rgb renderer.RGB) { | |
| 1060 | + | kindEvent := paletteEventForeground | |
| 1061 | + | if kind == 11 { | |
| 1062 | + | kindEvent = paletteEventBackground | |
| 1063 | + | } | |
| 1064 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: kindEvent, rgb: rgb}) | |
| 1065 | + | }, func(slot int, rgb renderer.RGB) { | |
| 1066 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventPalette, slot: uint8(slot), rgb: rgb}) | |
| 1067 | + | }, func(light bool) { | |
| 1068 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventScheme, light: light}) | |
| 1069 | + | }, func(data []byte) { | |
| 1070 | + | markers.scan(data, sink, func() { | |
| 1071 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventMarker}) | |
| 1072 | + | }) | |
| 1073 | + | }) | |
| 1074 | + | if !sendOK.Load() { | |
| 1075 | + | return | |
| 1076 | + | } | |
| 1077 | + | } | |
| 1078 | + | if markers.hasPendingPrefix() { | |
| 1079 | + | armMarkerDeadline() | |
| 1080 | + | } | |
| 1081 | + | if result.err != nil { | |
| 1082 | + | disarmMarkerDeadline() | |
| 1083 | + | markers.flush(sink) | |
| 1084 | + | select { | |
| 1085 | + | case p.out <- ports.Frame{Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}: | |
| 1086 | + | case <-p.ctx.Done(): | |
| 1087 | + | } | |
| 1088 | + | p.cancel() | |
| 1089 | + | return | |
| 1090 | + | } | |
| 1091 | + | } | |
| 1017 | 1092 | } | |
| 1018 | 1093 | } | |
| 1019 | 1094 | ||
| @@ -910,11 +910,11 @@ func runSender(ctx context.Context, cancel context.CancelFunc, transport ports.T | |||||
| 910 | // cancels the loop. | 910 | // cancels the loop. | ||
| 911 | // | 911 | // | ||
| 912 | // Known MVP limitation: a bare io.Reader cannot be unblocked from outside, | 912 | // Known MVP limitation: a bare io.Reader cannot be unblocked from outside, | ||
| 913 | - | // so on shutdown initiated elsewhere (daemon detach, transport loss) this | 913 | + | // so on shutdown initiated elsewhere (daemon detach, transport loss) the |
| 914 | - | // goroutine stays parked in Read until the next byte arrives or the process | 914 | + | // reader goroutine stays parked in Read until the next byte arrives or the |
| 915 | - | // exits. That is harmless here — Run has already returned and restored the | 915 | + | // process exits. That is harmless here — Run has already returned and |
| 916 | - | // terminal — and matches the standard pattern for stdin pumps; a | 916 | + | // restored the terminal — and matches the standard pattern for stdin pumps; |
| 917 | - | // closable stdin duplicate could lift it later if ever needed. | 917 | + | // a closable stdin duplicate could lift it later if ever needed. |
| 918 | // stdinPump parses terminal color reports into generation-tagged events while | 918 | // stdinPump parses terminal color reports into generation-tagged events while | ||
| 919 | // preserving all ordinary terminal input byte-for-byte. It never publishes a | 919 | // preserving all ordinary terminal input byte-for-byte. It never publishes a | ||
| 920 | // Theme and never writes terminal output; attachAttempt owns both operations. | 920 | // Theme and never writes terminal output; attachAttempt owns both operations. | ||
| @@ -978,42 +978,117 @@ func (p *stdinPump) run() { | |||||
| 978 | } | 978 | } | ||
| 979 | sink = ci.Scan | 979 | sink = ci.Scan | ||
| 980 | } | 980 | } | ||
| 981 | - | for { | 981 | + | type readResult struct { |
| 982 | - | n, rerr := p.in.Read(buf) | 982 | + | data []byte |
| 983 | - | // All scanner callbacks from this Read must retain this one generation. | 983 | + | err error |
| 984 | - | // A scheme notification can start its replacement before a later byte in | 984 | + | } |
| 985 | - | // the same read is scanned; reloading there would misclassify an old | 985 | + | reads := make(chan readResult, 1) |
| 986 | - | // completion marker as the replacement's drain response. | 986 | + | go func() { |
| 987 | - | readGeneration := paletteGenerationID(p.activeGeneration.Load()) | 987 | + | defer close(reads) |
| 988 | - | if n > 0 { | 988 | + | for { |
| 989 | - | scanner.Scan(buf[:n], func(kind int, rgb renderer.RGB) { | 989 | + | n, err := p.in.Read(buf) |
| 990 | - | kindEvent := paletteEventForeground | 990 | + | result := readResult{err: err} |
| 991 | - | if kind == 11 { | 991 | + | if n > 0 { |
| 992 | - | kindEvent = paletteEventBackground | 992 | + | result.data = append([]byte(nil), buf[:n]...) |
| 993 | - | } | |||
| 994 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: kindEvent, rgb: rgb}) | |||
| 995 | - | }, func(slot int, rgb renderer.RGB) { | |||
| 996 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventPalette, slot: uint8(slot), rgb: rgb}) | |||
| 997 | - | }, func(light bool) { | |||
| 998 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventScheme, light: light}) | |||
| 999 | - | }, func(data []byte) { | |||
| 1000 | - | markers.scan(data, sink, func() { | |||
| 1001 | - | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventMarker}) | |||
| 1002 | - | }) | |||
| 1003 | - | }) | |||
| 1004 | - | if !sendOK.Load() { | |||
| 1005 | - | return | |||
| 1006 | } | 993 | } | ||
| 1007 | - | } | |||
| 1008 | - | if rerr != nil { | |||
| 1009 | - | markers.flush(sink) | |||
| 1010 | select { | 994 | select { | ||
| 1011 | - | case p.out <- ports.Frame{Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}: | 995 | + | case reads <- result: |
| 1012 | case <-p.ctx.Done(): | 996 | case <-p.ctx.Done(): | ||
| 997 | + | return | |||
| 1013 | } | 998 | } | ||
| 1014 | - | p.cancel() | 999 | + | if err != nil { |
| 1000 | + | return | |||
| 1001 | + | } | |||
| 1002 | + | } | |||
| 1003 | + | }() | |||
| 1004 | + | ||||
| 1005 | + | var markerTimer ports.Timer | |||
| 1006 | + | var markerTimerC <-chan time.Time | |||
| 1007 | + | flushMarkerPrefix := func() { | |||
| 1008 | + | markers.flush(sink) | |||
| 1009 | + | markerTimer = nil | |||
| 1010 | + | markerTimerC = nil | |||
| 1011 | + | } | |||
| 1012 | + | // Disarm before processing a subsequent read. A deadline already ready at | |||
| 1013 | + | // that boundary wins, so bytes cannot be retroactively consumed as a | |||
| 1014 | + | // marker after they were due to be forwarded as ordinary input. | |||
| 1015 | + | disarmMarkerDeadline := func() { | |||
| 1016 | + | if markerTimer == nil { | |||
| 1017 | + | return | |||
| 1018 | + | } | |||
| 1019 | + | select { | |||
| 1020 | + | case <-markerTimerC: | |||
| 1021 | + | flushMarkerPrefix() | |||
| 1022 | + | return | |||
| 1023 | + | default: | |||
| 1024 | + | } | |||
| 1025 | + | if !markerTimer.Stop() { | |||
| 1026 | + | // Stop reports false only after expiry or a prior stop. This timer is | |||
| 1027 | + | // owned only here, so expiry wins even if delivery to C races this | |||
| 1028 | + | // select; do not let a late value consume a newly arrived suffix. | |||
| 1029 | + | flushMarkerPrefix() | |||
| 1015 | return | 1030 | return | ||
| 1016 | } | 1031 | } | ||
| 1032 | + | markerTimer = nil | |||
| 1033 | + | markerTimerC = nil | |||
| 1034 | + | } | |||
| 1035 | + | armMarkerDeadline := func() { | |||
| 1036 | + | disarmMarkerDeadline() | |||
| 1037 | + | markerTimer = p.clock.NewTimer(paletteMarkerAmbiguityDeadline) | |||
| 1038 | + | markerTimerC = markerTimer.C() | |||
| 1039 | + | } | |||
| 1040 | + | defer disarmMarkerDeadline() | |||
| 1041 | + | ||||
| 1042 | + | for { | |||
| 1043 | + | select { | |||
| 1044 | + | case <-p.ctx.Done(): | |||
| 1045 | + | return | |||
| 1046 | + | case <-markerTimerC: | |||
| 1047 | + | flushMarkerPrefix() | |||
| 1048 | + | case result, ok := <-reads: | |||
| 1049 | + | if !ok { | |||
| 1050 | + | return | |||
| 1051 | + | } | |||
| 1052 | + | disarmMarkerDeadline() | |||
| 1053 | + | // All scanner callbacks from this Read must retain this one generation. | |||
| 1054 | + | // A scheme notification can start its replacement before a later byte in | |||
| 1055 | + | // the same read is scanned; reloading there would misclassify an old | |||
| 1056 | + | // completion marker as the replacement's drain response. | |||
| 1057 | + | readGeneration := paletteGenerationID(p.activeGeneration.Load()) | |||
| 1058 | + | if len(result.data) > 0 { | |||
| 1059 | + | scanner.Scan(result.data, func(kind int, rgb renderer.RGB) { | |||
| 1060 | + | kindEvent := paletteEventForeground | |||
| 1061 | + | if kind == 11 { | |||
| 1062 | + | kindEvent = paletteEventBackground | |||
| 1063 | + | } | |||
| 1064 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: kindEvent, rgb: rgb}) | |||
| 1065 | + | }, func(slot int, rgb renderer.RGB) { | |||
| 1066 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventPalette, slot: uint8(slot), rgb: rgb}) | |||
| 1067 | + | }, func(light bool) { | |||
| 1068 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventScheme, light: light}) | |||
| 1069 | + | }, func(data []byte) { | |||
| 1070 | + | markers.scan(data, sink, func() { | |||
| 1071 | + | sendEvent(paletteGenerationEvent{id: readGeneration, kind: paletteEventMarker}) | |||
| 1072 | + | }) | |||
| 1073 | + | }) | |||
| 1074 | + | if !sendOK.Load() { | |||
| 1075 | + | return | |||
| 1076 | + | } | |||
| 1077 | + | } | |||
| 1078 | + | if markers.hasPendingPrefix() { | |||
| 1079 | + | armMarkerDeadline() | |||
| 1080 | + | } | |||
| 1081 | + | if result.err != nil { | |||
| 1082 | + | disarmMarkerDeadline() | |||
| 1083 | + | markers.flush(sink) | |||
| 1084 | + | select { | |||
| 1085 | + | case p.out <- ports.Frame{Type: ports.MsgDetach, Payload: ports.MarshalDetach(ports.Detach{})}: | |||
| 1086 | + | case <-p.ctx.Done(): | |||
| 1087 | + | } | |||
| 1088 | + | p.cancel() | |||
| 1089 | + | return | |||
| 1090 | + | } | |||
| 1091 | + | } | |||
| 1017 | } | 1092 | } | ||
| 1018 | } | 1093 | } | ||
| 1019 | 1094 | ||||
M internal/usecase/client/palette_attach_generation_test.go +51 -4
| @@ -22,7 +22,11 @@ import ( | |||
| 22 | 22 | // Tests fire only the deadline they intend to exercise; no wall clock is used. | |
| 23 | 23 | type attachPaletteClock struct{ timers chan *attachPaletteTimer } | |
| 24 | 24 | ||
| 25 | - | type attachPaletteTimer struct{ ch chan time.Time } | |
| 25 | + | type attachPaletteTimer struct { | |
| 26 | + | ch chan time.Time | |
| 27 | + | stopped chan struct{} | |
| 28 | + | stopOnce sync.Once | |
| 29 | + | } | |
| 26 | 30 | ||
| 27 | 31 | func newAttachPaletteClock() *attachPaletteClock { | |
| 28 | 32 | return &attachPaletteClock{timers: make(chan *attachPaletteTimer, 16)} | |
| @@ -30,14 +34,17 @@ func newAttachPaletteClock() *attachPaletteClock { | |||
| 30 | 34 | ||
| 31 | 35 | func (*attachPaletteClock) Now() time.Time { return time.Time{} } | |
| 32 | 36 | func (c *attachPaletteClock) NewTimer(time.Duration) ports.Timer { | |
| 33 | - | timer := &attachPaletteTimer{ch: make(chan time.Time, 1)} | |
| 37 | + | timer := &attachPaletteTimer{ch: make(chan time.Time, 1), stopped: make(chan struct{})} | |
| 34 | 38 | c.timers <- timer | |
| 35 | 39 | return timer | |
| 36 | 40 | } | |
| 37 | 41 | func (t *attachPaletteTimer) C() <-chan time.Time { return t.ch } | |
| 38 | 42 | func (*attachPaletteTimer) Reset(time.Duration) bool { return false } | |
| 39 | - | func (*attachPaletteTimer) Stop() bool { return true } | |
| 40 | - | func (t *attachPaletteTimer) fire() { t.ch <- time.Time{} } | |
| 43 | + | func (t *attachPaletteTimer) Stop() bool { | |
| 44 | + | t.stopOnce.Do(func() { close(t.stopped) }) | |
| 45 | + | return true | |
| 46 | + | } | |
| 47 | + | func (t *attachPaletteTimer) fire() { t.ch <- time.Time{} } | |
| 41 | 48 | ||
| 42 | 49 | type attachPaletteReader struct{ chunks chan []byte } | |
| 43 | 50 | ||
| @@ -370,6 +377,46 @@ func TestAttachLateDrainCannotFinalizeReplacementEarly(t *testing.T) { | |||
| 370 | 377 | h.detach(t) | |
| 371 | 378 | } | |
| 372 | 379 | ||
| 380 | + | func TestAttachForwardsStandaloneEscapeAfterAmbiguityDeadline(t *testing.T) { | |
| 381 | + | t.Setenv("COLORTERM", "truecolor") | |
| 382 | + | h := startAttachPaletteHarness(&terminalThemeState{}) | |
| 383 | + | require.Equal(t, paletteColorBatch, h.nextWrite(t)) | |
| 384 | + | _ = h.nextTimer(t) // initial palette completion deadline | |
| 385 | + | _ = h.nextTheme(t) // initial cleared publication | |
| 386 | + | ||
| 387 | + | // A bare Escape is ambiguous with the prefix of the DECRQM completion | |
| 388 | + | // response. It must not require another Read or EOF before reaching the | |
| 389 | + | // daemon. | |
| 390 | + | h.reader.send("\x1b") | |
| 391 | + | h.nextTimer(t).fire() // DECRQM ambiguity deadline | |
| 392 | + | h.nextTimer(t).fire() // bracketed-paste ambiguity deadline | |
| 393 | + | input := <-h.transport.inputCh | |
| 394 | + | require.Equal(t, []byte("\x1b"), input.Data) | |
| 395 | + | h.detach(t) | |
| 396 | + | } | |
| 397 | + | ||
| 398 | + | func TestAttachConsumesSplitMarkerAndCancelsAmbiguityDeadline(t *testing.T) { | |
| 399 | + | t.Setenv("COLORTERM", "truecolor") | |
| 400 | + | h := startAttachPaletteHarness(&terminalThemeState{}) | |
| 401 | + | require.Equal(t, paletteColorBatch, h.nextWrite(t)) | |
| 402 | + | _ = h.nextTimer(t) // initial palette completion deadline | |
| 403 | + | _ = h.nextTheme(t) // initial cleared publication | |
| 404 | + | ||
| 405 | + | h.reader.send("before\x1b[?2031;") | |
| 406 | + | markerDeadline := h.nextTimer(t) | |
| 407 | + | h.reader.send("1$yafter") | |
| 408 | + | ||
| 409 | + | final := h.nextTheme(t) | |
| 410 | + | require.False(t, final.HasForeground) | |
| 411 | + | var got []byte | |
| 412 | + | for len(got) < len("beforeafter") { | |
| 413 | + | got = append(got, (<-h.transport.inputCh).Data...) | |
| 414 | + | } | |
| 415 | + | require.Equal(t, []byte("beforeafter"), got) | |
| 416 | + | <-markerDeadline.stopped | |
| 417 | + | h.detach(t) | |
| 418 | + | } | |
| 419 | + | ||
| 373 | 420 | func TestAttachForwardsInputOnceAndPartialOSCDoesNotPublishTheme(t *testing.T) { | |
| 374 | 421 | t.Setenv("COLORTERM", "truecolor") | |
| 375 | 422 | h := startAttachPaletteHarness(&terminalThemeState{}) | |
| @@ -22,7 +22,11 @@ import ( | |||||
| 22 | // Tests fire only the deadline they intend to exercise; no wall clock is used. | 22 | // Tests fire only the deadline they intend to exercise; no wall clock is used. | ||
| 23 | type attachPaletteClock struct{ timers chan *attachPaletteTimer } | 23 | type attachPaletteClock struct{ timers chan *attachPaletteTimer } | ||
| 24 | 24 | ||||
| 25 | - | type attachPaletteTimer struct{ ch chan time.Time } | 25 | + | type attachPaletteTimer struct { |
| 26 | + | ch chan time.Time | |||
| 27 | + | stopped chan struct{} | |||
| 28 | + | stopOnce sync.Once | |||
| 29 | + | } | |||
| 26 | 30 | ||||
| 27 | func newAttachPaletteClock() *attachPaletteClock { | 31 | func newAttachPaletteClock() *attachPaletteClock { | ||
| 28 | return &attachPaletteClock{timers: make(chan *attachPaletteTimer, 16)} | 32 | return &attachPaletteClock{timers: make(chan *attachPaletteTimer, 16)} | ||
| @@ -30,14 +34,17 @@ func newAttachPaletteClock() *attachPaletteClock { | |||||
| 30 | 34 | ||||
| 31 | func (*attachPaletteClock) Now() time.Time { return time.Time{} } | 35 | func (*attachPaletteClock) Now() time.Time { return time.Time{} } | ||
| 32 | func (c *attachPaletteClock) NewTimer(time.Duration) ports.Timer { | 36 | func (c *attachPaletteClock) NewTimer(time.Duration) ports.Timer { | ||
| 33 | - | timer := &attachPaletteTimer{ch: make(chan time.Time, 1)} | 37 | + | timer := &attachPaletteTimer{ch: make(chan time.Time, 1), stopped: make(chan struct{})} |
| 34 | c.timers <- timer | 38 | c.timers <- timer | ||
| 35 | return timer | 39 | return timer | ||
| 36 | } | 40 | } | ||
| 37 | func (t *attachPaletteTimer) C() <-chan time.Time { return t.ch } | 41 | func (t *attachPaletteTimer) C() <-chan time.Time { return t.ch } | ||
| 38 | func (*attachPaletteTimer) Reset(time.Duration) bool { return false } | 42 | func (*attachPaletteTimer) Reset(time.Duration) bool { return false } | ||
| 39 | - | func (*attachPaletteTimer) Stop() bool { return true } | 43 | + | func (t *attachPaletteTimer) Stop() bool { |
| 40 | - | func (t *attachPaletteTimer) fire() { t.ch <- time.Time{} } | 44 | + | t.stopOnce.Do(func() { close(t.stopped) }) |
| 45 | + | return true | |||
| 46 | + | } | |||
| 47 | + | func (t *attachPaletteTimer) fire() { t.ch <- time.Time{} } | |||
| 41 | 48 | ||||
| 42 | type attachPaletteReader struct{ chunks chan []byte } | 49 | type attachPaletteReader struct{ chunks chan []byte } | ||
| 43 | 50 | ||||
| @@ -370,6 +377,46 @@ func TestAttachLateDrainCannotFinalizeReplacementEarly(t *testing.T) { | |||||
| 370 | h.detach(t) | 377 | h.detach(t) | ||
| 371 | } | 378 | } | ||
| 372 | 379 | ||||
| 380 | + | func TestAttachForwardsStandaloneEscapeAfterAmbiguityDeadline(t *testing.T) { | |||
| 381 | + | t.Setenv("COLORTERM", "truecolor") | |||
| 382 | + | h := startAttachPaletteHarness(&terminalThemeState{}) | |||
| 383 | + | require.Equal(t, paletteColorBatch, h.nextWrite(t)) | |||
| 384 | + | _ = h.nextTimer(t) // initial palette completion deadline | |||
| 385 | + | _ = h.nextTheme(t) // initial cleared publication | |||
| 386 | + | ||||
| 387 | + | // A bare Escape is ambiguous with the prefix of the DECRQM completion | |||
| 388 | + | // response. It must not require another Read or EOF before reaching the | |||
| 389 | + | // daemon. | |||
| 390 | + | h.reader.send("\x1b") | |||
| 391 | + | h.nextTimer(t).fire() // DECRQM ambiguity deadline | |||
| 392 | + | h.nextTimer(t).fire() // bracketed-paste ambiguity deadline | |||
| 393 | + | input := <-h.transport.inputCh | |||
| 394 | + | require.Equal(t, []byte("\x1b"), input.Data) | |||
| 395 | + | h.detach(t) | |||
| 396 | + | } | |||
| 397 | + | ||||
| 398 | + | func TestAttachConsumesSplitMarkerAndCancelsAmbiguityDeadline(t *testing.T) { | |||
| 399 | + | t.Setenv("COLORTERM", "truecolor") | |||
| 400 | + | h := startAttachPaletteHarness(&terminalThemeState{}) | |||
| 401 | + | require.Equal(t, paletteColorBatch, h.nextWrite(t)) | |||
| 402 | + | _ = h.nextTimer(t) // initial palette completion deadline | |||
| 403 | + | _ = h.nextTheme(t) // initial cleared publication | |||
| 404 | + | ||||
| 405 | + | h.reader.send("before\x1b[?2031;") | |||
| 406 | + | markerDeadline := h.nextTimer(t) | |||
| 407 | + | h.reader.send("1$yafter") | |||
| 408 | + | ||||
| 409 | + | final := h.nextTheme(t) | |||
| 410 | + | require.False(t, final.HasForeground) | |||
| 411 | + | var got []byte | |||
| 412 | + | for len(got) < len("beforeafter") { | |||
| 413 | + | got = append(got, (<-h.transport.inputCh).Data...) | |||
| 414 | + | } | |||
| 415 | + | require.Equal(t, []byte("beforeafter"), got) | |||
| 416 | + | <-markerDeadline.stopped | |||
| 417 | + | h.detach(t) | |||
| 418 | + | } | |||
| 419 | + | ||||
| 373 | func TestAttachForwardsInputOnceAndPartialOSCDoesNotPublishTheme(t *testing.T) { | 420 | func TestAttachForwardsInputOnceAndPartialOSCDoesNotPublishTheme(t *testing.T) { | ||
| 374 | t.Setenv("COLORTERM", "truecolor") | 421 | t.Setenv("COLORTERM", "truecolor") | ||
| 375 | h := startAttachPaletteHarness(&terminalThemeState{}) | 422 | h := startAttachPaletteHarness(&terminalThemeState{}) | ||
M internal/usecase/client/palette_generation.go +7 -0
| @@ -19,6 +19,11 @@ const paletteColorBatch = "\x1b]10;?\x07\x1b]11;?\x07" + | |||
| 19 | 19 | // user-facing timeout. Timer ownership belongs to the attach loop. | |
| 20 | 20 | const paletteGenerationDeadline = 200 * time.Millisecond | |
| 21 | 21 | ||
| 22 | + | // paletteMarkerAmbiguityDeadline bounds how long an Escape byte may remain | |
| 23 | + | // withheld while it could still become a DECRQM completion response. Match | |
| 24 | + | // the existing local input-prefix bound so a keypress remains prompt. | |
| 25 | + | const paletteMarkerAmbiguityDeadline = pasteFlushDelay | |
| 26 | + | ||
| 22 | 27 | type paletteGenerationID uint64 | |
| 23 | 28 | ||
| 24 | 29 | type paletteGenerationPhase uint8 | |
| @@ -302,3 +307,5 @@ func (s *paletteMarkerScanner) flush(onBytes func([]byte)) { | |||
| 302 | 307 | onBytes(s.pending) | |
| 303 | 308 | s.pending = nil | |
| 304 | 309 | } | |
| 310 | + | ||
| 311 | + | func (s *paletteMarkerScanner) hasPendingPrefix() bool { return len(s.pending) != 0 } | |
| @@ -19,6 +19,11 @@ const paletteColorBatch = "\x1b]10;?\x07\x1b]11;?\x07" + | |||||
| 19 | // user-facing timeout. Timer ownership belongs to the attach loop. | 19 | // user-facing timeout. Timer ownership belongs to the attach loop. | ||
| 20 | const paletteGenerationDeadline = 200 * time.Millisecond | 20 | const paletteGenerationDeadline = 200 * time.Millisecond | ||
| 21 | 21 | ||||
| 22 | + | // paletteMarkerAmbiguityDeadline bounds how long an Escape byte may remain | |||
| 23 | + | // withheld while it could still become a DECRQM completion response. Match | |||
| 24 | + | // the existing local input-prefix bound so a keypress remains prompt. | |||
| 25 | + | const paletteMarkerAmbiguityDeadline = pasteFlushDelay | |||
| 26 | + | ||||
| 22 | type paletteGenerationID uint64 | 27 | type paletteGenerationID uint64 | ||
| 23 | 28 | ||||
| 24 | type paletteGenerationPhase uint8 | 29 | type paletteGenerationPhase uint8 | ||
| @@ -302,3 +307,5 @@ func (s *paletteMarkerScanner) flush(onBytes func([]byte)) { | |||||
| 302 | onBytes(s.pending) | 307 | onBytes(s.pending) | ||
| 303 | s.pending = nil | 308 | s.pending = nil | ||
| 304 | } | 309 | } | ||
| 310 | + | ||||
| 311 | + | func (s *paletteMarkerScanner) hasPendingPrefix() bool { return len(s.pending) != 0 } | |||