fix(daemon): handle final notification edge cases
- commit
- ed50af9ccca99cc8a76991726bec2ff0ca882832
- author
- brice · · 2026-07-21 12:58 UTC
- parent
- 4eadd47
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
9 files · +148 -28
M internal/adapters/noticefile/noticefile_test.go +4 -3
| @@ -127,9 +127,10 @@ func TestStoreClaimSkipsOversizedLines(t *testing.T) { | |||
| 127 | 127 | require.NoError(t, err) | |
| 128 | 128 | return string(data) | |
| 129 | 129 | } | |
| 130 | - | const jsonMessagePrefix = `{"Code":0,"Severity":0,"Message":"` | |
| 131 | - | const jsonMessageSuffix = `","Details":"","Time":"0001-01-01T00:00:00Z","Count":0,"SessionID":""}` | |
| 132 | - | boundaryMessage := strings.Repeat("x", maxNoticeRecordSize-len(jsonMessagePrefix)-len(jsonMessageSuffix)) | |
| 130 | + | // Derive JSON overhead from the encoder rather than duplicating its current | |
| 131 | + | // field layout here. The boundary payload uses ASCII so each added message | |
| 132 | + | // byte remains one marshaled byte. | |
| 133 | + | boundaryMessage := strings.Repeat("x", maxNoticeRecordSize-len(validLine(""))) | |
| 133 | 134 | ||
| 134 | 135 | for _, tt := range []struct { | |
| 135 | 136 | name string | |
| @@ -127,9 +127,10 @@ func TestStoreClaimSkipsOversizedLines(t *testing.T) { | |||||
| 127 | require.NoError(t, err) | 127 | require.NoError(t, err) | ||
| 128 | return string(data) | 128 | return string(data) | ||
| 129 | } | 129 | } | ||
| 130 | - | const jsonMessagePrefix = `{"Code":0,"Severity":0,"Message":"` | 130 | + | // Derive JSON overhead from the encoder rather than duplicating its current |
| 131 | - | const jsonMessageSuffix = `","Details":"","Time":"0001-01-01T00:00:00Z","Count":0,"SessionID":""}` | 131 | + | // field layout here. The boundary payload uses ASCII so each added message |
| 132 | - | boundaryMessage := strings.Repeat("x", maxNoticeRecordSize-len(jsonMessagePrefix)-len(jsonMessageSuffix)) | 132 | + | // byte remains one marshaled byte. |
| 133 | + | boundaryMessage := strings.Repeat("x", maxNoticeRecordSize-len(validLine(""))) | |||
| 133 | 134 | ||||
| 134 | for _, tt := range []struct { | 135 | for _, tt := range []struct { | ||
| 135 | name string | 136 | name string | ||
M internal/usecase/daemon/notices_overlay.go +26 -13
| @@ -73,9 +73,16 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||
| 73 | 73 | result.exit = true | |
| 74 | 74 | case keys.ESC: | |
| 75 | 75 | tail := data[i:] | |
| 76 | - | if consumed, ok := routeListEscape(state, tail); ok { | |
| 76 | + | if consumed, move := routeListEscape(tail); consumed > 0 { | |
| 77 | + | switch move { | |
| 78 | + | case 'A': | |
| 79 | + | state.moveUp() | |
| 80 | + | result.changed = true | |
| 81 | + | case 'B': | |
| 82 | + | state.moveDown() | |
| 83 | + | result.changed = true | |
| 84 | + | } | |
| 77 | 85 | i += consumed - 1 | |
| 78 | - | result.changed = true | |
| 79 | 86 | continue | |
| 80 | 87 | } | |
| 81 | 88 | if len(tail) == 1 { | |
| @@ -103,22 +110,28 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||
| 103 | 110 | return result | |
| 104 | 111 | } | |
| 105 | 112 | ||
| 106 | - | func routeListEscape(state listInputState, data []byte) (int, bool) { | |
| 107 | - | if len(data) >= 3 && (data[1] == '[' || data[1] == 'O') { | |
| 108 | - | switch data[2] { | |
| 109 | - | case 'A': | |
| 110 | - | state.moveUp() | |
| 111 | - | return 3, true | |
| 112 | - | case 'B': | |
| 113 | - | state.moveDown() | |
| 114 | - | return 3, true | |
| 113 | + | // routeListEscape consumes one complete CSI or SS3 sequence. It returns its | |
| 114 | + | // byte length and the simple arrow final when it is an Up/Down sequence. Other | |
| 115 | + | // complete terminal sequences are deliberately ignored so their bytes cannot | |
| 116 | + | // trigger list actions or close the overlay. | |
| 117 | + | func routeListEscape(data []byte) (consumed int, move byte) { | |
| 118 | + | if len(data) < 2 || data[0] != keys.ESC || (data[1] != '[' && data[1] != 'O') { | |
| 119 | + | return 0, 0 | |
| 120 | + | } | |
| 121 | + | for i := 2; i < len(data); i++ { | |
| 122 | + | if data[i] < 0x40 || data[i] > 0x7e { | |
| 123 | + | continue | |
| 124 | + | } | |
| 125 | + | if i == 2 && (data[i] == 'A' || data[i] == 'B') { | |
| 126 | + | return i + 1, data[i] | |
| 115 | 127 | } | |
| 128 | + | return i + 1, 0 | |
| 116 | 129 | } | |
| 117 | - | return 0, false | |
| 130 | + | return 0, 0 | |
| 118 | 131 | } | |
| 119 | 132 | ||
| 120 | 133 | func isListEscapePrefix(data []byte) bool { | |
| 121 | - | return len(data) == 2 && data[0] == keys.ESC && (data[1] == '[' || data[1] == 'O') | |
| 134 | + | return len(data) >= 2 && data[0] == keys.ESC && (data[1] == '[' || data[1] == 'O') | |
| 122 | 135 | } | |
| 123 | 136 | ||
| 124 | 137 | func retainListESCLocked(clock ports.Clock, state listInputState) { | |
| @@ -73,9 +73,16 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||||
| 73 | result.exit = true | 73 | result.exit = true | ||
| 74 | case keys.ESC: | 74 | case keys.ESC: | ||
| 75 | tail := data[i:] | 75 | tail := data[i:] | ||
| 76 | - | if consumed, ok := routeListEscape(state, tail); ok { | 76 | + | if consumed, move := routeListEscape(tail); consumed > 0 { |
| 77 | + | switch move { | |||
| 78 | + | case 'A': | |||
| 79 | + | state.moveUp() | |||
| 80 | + | result.changed = true | |||
| 81 | + | case 'B': | |||
| 82 | + | state.moveDown() | |||
| 83 | + | result.changed = true | |||
| 84 | + | } | |||
| 77 | i += consumed - 1 | 85 | i += consumed - 1 | ||
| 78 | - | result.changed = true | |||
| 79 | continue | 86 | continue | ||
| 80 | } | 87 | } | ||
| 81 | if len(tail) == 1 { | 88 | if len(tail) == 1 { | ||
| @@ -103,22 +110,28 @@ func handleListInputLocked(clock ports.Clock, data []byte, state listInputState, | |||||
| 103 | return result | 110 | return result | ||
| 104 | } | 111 | } | ||
| 105 | 112 | ||||
| 106 | - | func routeListEscape(state listInputState, data []byte) (int, bool) { | 113 | + | // routeListEscape consumes one complete CSI or SS3 sequence. It returns its |
| 107 | - | if len(data) >= 3 && (data[1] == '[' || data[1] == 'O') { | 114 | + | // byte length and the simple arrow final when it is an Up/Down sequence. Other |
| 108 | - | switch data[2] { | 115 | + | // complete terminal sequences are deliberately ignored so their bytes cannot |
| 109 | - | case 'A': | 116 | + | // trigger list actions or close the overlay. |
| 110 | - | state.moveUp() | 117 | + | func routeListEscape(data []byte) (consumed int, move byte) { |
| 111 | - | return 3, true | 118 | + | if len(data) < 2 || data[0] != keys.ESC || (data[1] != '[' && data[1] != 'O') { |
| 112 | - | case 'B': | 119 | + | return 0, 0 |
| 113 | - | state.moveDown() | 120 | + | } |
| 114 | - | return 3, true | 121 | + | for i := 2; i < len(data); i++ { |
| 122 | + | if data[i] < 0x40 || data[i] > 0x7e { | |||
| 123 | + | continue | |||
| 124 | + | } | |||
| 125 | + | if i == 2 && (data[i] == 'A' || data[i] == 'B') { | |||
| 126 | + | return i + 1, data[i] | |||
| 115 | } | 127 | } | ||
| 128 | + | return i + 1, 0 | |||
| 116 | } | 129 | } | ||
| 117 | - | return 0, false | 130 | + | return 0, 0 |
| 118 | } | 131 | } | ||
| 119 | 132 | ||||
| 120 | func isListEscapePrefix(data []byte) bool { | 133 | func isListEscapePrefix(data []byte) bool { | ||
| 121 | - | return len(data) == 2 && data[0] == keys.ESC && (data[1] == '[' || data[1] == 'O') | 134 | + | return len(data) >= 2 && data[0] == keys.ESC && (data[1] == '[' || data[1] == 'O') |
| 122 | } | 135 | } | ||
| 123 | 136 | ||||
| 124 | func retainListESCLocked(clock ports.Clock, state listInputState) { | 137 | func retainListESCLocked(clock ports.Clock, state listInputState) { | ||
M internal/usecase/daemon/notices_overlay_test.go +37 -0
| @@ -185,6 +185,43 @@ func TestHandleListInputConsumesSplitEscapePrefix(t *testing.T) { | |||
| 185 | 185 | } | |
| 186 | 186 | } | |
| 187 | 187 | ||
| 188 | + | func TestHandleListInputConsumesUnsupportedCompleteEscapeSequences(t *testing.T) { | |
| 189 | + | for _, tt := range []struct { | |
| 190 | + | name string | |
| 191 | + | input [][]byte | |
| 192 | + | }{ | |
| 193 | + | {name: "complete CSI right", input: [][]byte{[]byte("\x1b[C")}}, | |
| 194 | + | {name: "complete SS3 left", input: [][]byte{[]byte("\x1bOD")}}, | |
| 195 | + | {name: "parameterized CSI", input: [][]byte{[]byte("\x1b[1;5C")}}, | |
| 196 | + | {name: "split CSI right", input: [][]byte{[]byte("\x1b["), []byte("C")}}, | |
| 197 | + | {name: "split SS3 left", input: [][]byte{[]byte("\x1bO"), []byte("D")}}, | |
| 198 | + | } { | |
| 199 | + | t.Run(tt.name, func(t *testing.T) { | |
| 200 | + | var pending, custom []byte | |
| 201 | + | state := listInputState{ | |
| 202 | + | pending: &pending, | |
| 203 | + | esc: &pendingByteTimer{}, | |
| 204 | + | moveUp: func() {}, | |
| 205 | + | moveDown: func() {}, | |
| 206 | + | } | |
| 207 | + | action := func(b byte) listInputResult { | |
| 208 | + | custom = append(custom, b) | |
| 209 | + | return listInputResult{action: b} | |
| 210 | + | } | |
| 211 | + | ||
| 212 | + | var result listInputResult | |
| 213 | + | for _, input := range tt.input { | |
| 214 | + | result = handleListInputLocked(nil, input, state, action) | |
| 215 | + | } | |
| 216 | + | ||
| 217 | + | require.False(t, result.exit) | |
| 218 | + | require.Zero(t, result.action) | |
| 219 | + | require.Empty(t, custom, "complete escape bytes must not reach the custom action") | |
| 220 | + | require.Empty(t, pending) | |
| 221 | + | }) | |
| 222 | + | } | |
| 223 | + | } | |
| 224 | + | ||
| 188 | 225 | func TestNoticesOpenWithEmptyHistoryShowsPlaceholderWithoutPanic(t *testing.T) { | |
| 189 | 226 | p, release := newBlockingPTY(t) | |
| 190 | 227 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| @@ -185,6 +185,43 @@ func TestHandleListInputConsumesSplitEscapePrefix(t *testing.T) { | |||||
| 185 | } | 185 | } | ||
| 186 | } | 186 | } | ||
| 187 | 187 | ||||
| 188 | + | func TestHandleListInputConsumesUnsupportedCompleteEscapeSequences(t *testing.T) { | |||
| 189 | + | for _, tt := range []struct { | |||
| 190 | + | name string | |||
| 191 | + | input [][]byte | |||
| 192 | + | }{ | |||
| 193 | + | {name: "complete CSI right", input: [][]byte{[]byte("\x1b[C")}}, | |||
| 194 | + | {name: "complete SS3 left", input: [][]byte{[]byte("\x1bOD")}}, | |||
| 195 | + | {name: "parameterized CSI", input: [][]byte{[]byte("\x1b[1;5C")}}, | |||
| 196 | + | {name: "split CSI right", input: [][]byte{[]byte("\x1b["), []byte("C")}}, | |||
| 197 | + | {name: "split SS3 left", input: [][]byte{[]byte("\x1bO"), []byte("D")}}, | |||
| 198 | + | } { | |||
| 199 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 200 | + | var pending, custom []byte | |||
| 201 | + | state := listInputState{ | |||
| 202 | + | pending: &pending, | |||
| 203 | + | esc: &pendingByteTimer{}, | |||
| 204 | + | moveUp: func() {}, | |||
| 205 | + | moveDown: func() {}, | |||
| 206 | + | } | |||
| 207 | + | action := func(b byte) listInputResult { | |||
| 208 | + | custom = append(custom, b) | |||
| 209 | + | return listInputResult{action: b} | |||
| 210 | + | } | |||
| 211 | + | ||||
| 212 | + | var result listInputResult | |||
| 213 | + | for _, input := range tt.input { | |||
| 214 | + | result = handleListInputLocked(nil, input, state, action) | |||
| 215 | + | } | |||
| 216 | + | ||||
| 217 | + | require.False(t, result.exit) | |||
| 218 | + | require.Zero(t, result.action) | |||
| 219 | + | require.Empty(t, custom, "complete escape bytes must not reach the custom action") | |||
| 220 | + | require.Empty(t, pending) | |||
| 221 | + | }) | |||
| 222 | + | } | |||
| 223 | + | } | |||
| 224 | + | ||||
| 188 | func TestNoticesOpenWithEmptyHistoryShowsPlaceholderWithoutPanic(t *testing.T) { | 225 | func TestNoticesOpenWithEmptyHistoryShowsPlaceholderWithoutPanic(t *testing.T) { | ||
| 189 | p, release := newBlockingPTY(t) | 226 | p, release := newBlockingPTY(t) | ||
| 190 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | 227 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | ||
M internal/usecase/daemon/render_capture.go +1 -1
| @@ -332,7 +332,7 @@ func cloneLayoutNodeIntoScratch(src *layout.Node, scratch *renderCaptureScratch) | |||
| 332 | 332 | ||
| 333 | 333 | func captureCursorInputsLocked(p *pane, content domain.Rect, overlays capturedOverlayRenderState) capturedCursorInputs { | |
| 334 | 334 | style, hasStyle := p.screen.CursorStyle() | |
| 335 | - | hidden := overlays.copyActive || overlays.copySearchActive || overlays.pickerActive || overlays.paletteActive || overlays.promptActive | |
| 335 | + | hidden := overlays.copyActive || overlays.copySearchActive || overlays.pickerActive || overlays.paletteActive || overlays.promptActive || overlays.noticesOverlayActive | |
| 336 | 336 | return capturedCursorInputs{row: p.screen.CursorRow(), col: p.screen.CursorCol(), style: style, hasStyle: hasStyle, visible: p.screen.CursorVisible(), renderable: content.Width > 0 && content.Height > 0, hiddenByOverlay: hidden, content: content} | |
| 337 | 337 | } | |
| 338 | 338 | ||
| @@ -332,7 +332,7 @@ func cloneLayoutNodeIntoScratch(src *layout.Node, scratch *renderCaptureScratch) | |||||
| 332 | 332 | ||||
| 333 | func captureCursorInputsLocked(p *pane, content domain.Rect, overlays capturedOverlayRenderState) capturedCursorInputs { | 333 | func captureCursorInputsLocked(p *pane, content domain.Rect, overlays capturedOverlayRenderState) capturedCursorInputs { | ||
| 334 | style, hasStyle := p.screen.CursorStyle() | 334 | style, hasStyle := p.screen.CursorStyle() | ||
| 335 | - | hidden := overlays.copyActive || overlays.copySearchActive || overlays.pickerActive || overlays.paletteActive || overlays.promptActive | 335 | + | hidden := overlays.copyActive || overlays.copySearchActive || overlays.pickerActive || overlays.paletteActive || overlays.promptActive || overlays.noticesOverlayActive |
| 336 | return capturedCursorInputs{row: p.screen.CursorRow(), col: p.screen.CursorCol(), style: style, hasStyle: hasStyle, visible: p.screen.CursorVisible(), renderable: content.Width > 0 && content.Height > 0, hiddenByOverlay: hidden, content: content} | 336 | return capturedCursorInputs{row: p.screen.CursorRow(), col: p.screen.CursorCol(), style: style, hasStyle: hasStyle, visible: p.screen.CursorVisible(), renderable: content.Width > 0 && content.Height > 0, hiddenByOverlay: hidden, content: content} | ||
| 337 | } | 337 | } | ||
| 338 | 338 | ||||
M internal/usecase/daemon/render_damage_receipt_test.go +25 -0
| @@ -176,6 +176,31 @@ func TestPrepareFailureNotifiesAndSchedulesRecovery(t *testing.T) { | |||
| 176 | 176 | require.Contains(t, clock.durations(), urgentRenderDeadline, "notice/recovery invalidation must arm the coordinator") | |
| 177 | 177 | } | |
| 178 | 178 | ||
| 179 | + | func TestPrepareFailureFallbackOnlySuppressesRecursiveNoticePaint(t *testing.T) { | |
| 180 | + | d, sess, ac, sends := newNoticeFixture(t, newNoticeClock()) | |
| 181 | + | ||
| 182 | + | // A failure while reportError is repainting its own toast is intentionally | |
| 183 | + | // ignored. It must not start another notification/repaint cycle. | |
| 184 | + | ac.prepareFailureFallback.Store(true) | |
| 185 | + | state, composed := captureComposeForReceiptTest(t, sess, ac) | |
| 186 | + | composed.frame = renderer.Frame{Width: 1} | |
| 187 | + | require.True(t, d.emitFrame(sess, ac, state, composed)) | |
| 188 | + | require.Empty(t, d.notices.history()) | |
| 189 | + | ac.prepareFailureFallback.Store(false) | |
| 190 | + | ||
| 191 | + | // Later outer failures each notify and repaint after sendMu has been | |
| 192 | + | // released; neither may be hidden by the prior recursive guard. | |
| 193 | + | for want := 1; want <= 2; want++ { | |
| 194 | + | state, composed = captureComposeForReceiptTest(t, sess, ac) | |
| 195 | + | composed.frame = renderer.Frame{Width: 1} | |
| 196 | + | require.True(t, d.emitFrame(sess, ac, state, composed)) | |
| 197 | + | require.Len(t, d.notices.history(), want) | |
| 198 | + | frame := <-sends | |
| 199 | + | require.Equal(t, ports.MsgOutput, frame.Type, "outer failure %d must repaint its notice", want) | |
| 200 | + | require.False(t, ac.prepareFailureFallback.Load()) | |
| 201 | + | } | |
| 202 | + | } | |
| 203 | + | ||
| 179 | 204 | func TestRenderDamageReceiptStaleGenerationForcesFullRedraw(t *testing.T) { | |
| 180 | 205 | d, sess, ac, sends := newManualSessionWithPTYs(t, nil) | |
| 181 | 206 | d.paint(sess, ac, true, nil) | |
| @@ -176,6 +176,31 @@ func TestPrepareFailureNotifiesAndSchedulesRecovery(t *testing.T) { | |||||
| 176 | require.Contains(t, clock.durations(), urgentRenderDeadline, "notice/recovery invalidation must arm the coordinator") | 176 | require.Contains(t, clock.durations(), urgentRenderDeadline, "notice/recovery invalidation must arm the coordinator") | ||
| 177 | } | 177 | } | ||
| 178 | 178 | ||||
| 179 | + | func TestPrepareFailureFallbackOnlySuppressesRecursiveNoticePaint(t *testing.T) { | |||
| 180 | + | d, sess, ac, sends := newNoticeFixture(t, newNoticeClock()) | |||
| 181 | + | ||||
| 182 | + | // A failure while reportError is repainting its own toast is intentionally | |||
| 183 | + | // ignored. It must not start another notification/repaint cycle. | |||
| 184 | + | ac.prepareFailureFallback.Store(true) | |||
| 185 | + | state, composed := captureComposeForReceiptTest(t, sess, ac) | |||
| 186 | + | composed.frame = renderer.Frame{Width: 1} | |||
| 187 | + | require.True(t, d.emitFrame(sess, ac, state, composed)) | |||
| 188 | + | require.Empty(t, d.notices.history()) | |||
| 189 | + | ac.prepareFailureFallback.Store(false) | |||
| 190 | + | ||||
| 191 | + | // Later outer failures each notify and repaint after sendMu has been | |||
| 192 | + | // released; neither may be hidden by the prior recursive guard. | |||
| 193 | + | for want := 1; want <= 2; want++ { | |||
| 194 | + | state, composed = captureComposeForReceiptTest(t, sess, ac) | |||
| 195 | + | composed.frame = renderer.Frame{Width: 1} | |||
| 196 | + | require.True(t, d.emitFrame(sess, ac, state, composed)) | |||
| 197 | + | require.Len(t, d.notices.history(), want) | |||
| 198 | + | frame := <-sends | |||
| 199 | + | require.Equal(t, ports.MsgOutput, frame.Type, "outer failure %d must repaint its notice", want) | |||
| 200 | + | require.False(t, ac.prepareFailureFallback.Load()) | |||
| 201 | + | } | |||
| 202 | + | } | |||
| 203 | + | ||||
| 179 | func TestRenderDamageReceiptStaleGenerationForcesFullRedraw(t *testing.T) { | 204 | func TestRenderDamageReceiptStaleGenerationForcesFullRedraw(t *testing.T) { | ||
| 180 | d, sess, ac, sends := newManualSessionWithPTYs(t, nil) | 205 | d, sess, ac, sends := newManualSessionWithPTYs(t, nil) | ||
| 181 | d.paint(sess, ac, true, nil) | 206 | d.paint(sess, ac, true, nil) | ||
M internal/usecase/daemon/render_pipeline.go +9 -8
| @@ -428,16 +428,17 @@ func (d *Daemon) emitFrame(sess *session, ac *attachedClient, state *capturedRen | |||
| 428 | 428 | ac.sess.mu.Unlock() | |
| 429 | 429 | ac.sendMu.Unlock() | |
| 430 | 430 | d.log.Error("render draw failed", "err", err, "session", sess.name) | |
| 431 | - | // Without a coordinator invalidateRender paints synchronously. Suppress | |
| 432 | - | // the nested failed paint caused by reportError's notice repaint; the | |
| 433 | - | // outer failure still records the notice and attempts recovery once. | |
| 434 | - | fallback := sess.renderCoordinator() == nil | |
| 435 | - | if fallback && !ac.prepareFailureFallback.CompareAndSwap(false, true) { | |
| 431 | + | // Without a coordinator reportError repaints synchronously. Suppress only | |
| 432 | + | // that nested notice repaint; leave the guard before returning so a later, | |
| 433 | + | // independent failed transaction can still notify the user. | |
| 434 | + | if sess.renderCoordinator() == nil { | |
| 435 | + | if !ac.prepareFailureFallback.CompareAndSwap(false, true) { | |
| 436 | + | return true | |
| 437 | + | } | |
| 438 | + | d.reportError(sess, domain.UserErr(domain.NoticeInternal, "display update failed", err)) | |
| 439 | + | ac.prepareFailureFallback.Store(false) | |
| 436 | 440 | return true | |
| 437 | 441 | } | |
| 438 | - | if fallback { | |
| 439 | - | defer ac.prepareFailureFallback.Store(false) | |
| 440 | - | } | |
| 441 | 442 | d.reportError(sess, domain.UserErr(domain.NoticeInternal, "display update failed", err)) | |
| 442 | 443 | d.invalidateRender(sess, ac, true, "render_pipeline.go:prepare-failed") | |
| 443 | 444 | return true | |
| @@ -428,16 +428,17 @@ func (d *Daemon) emitFrame(sess *session, ac *attachedClient, state *capturedRen | |||||
| 428 | ac.sess.mu.Unlock() | 428 | ac.sess.mu.Unlock() | ||
| 429 | ac.sendMu.Unlock() | 429 | ac.sendMu.Unlock() | ||
| 430 | d.log.Error("render draw failed", "err", err, "session", sess.name) | 430 | d.log.Error("render draw failed", "err", err, "session", sess.name) | ||
| 431 | - | // Without a coordinator invalidateRender paints synchronously. Suppress | 431 | + | // Without a coordinator reportError repaints synchronously. Suppress only |
| 432 | - | // the nested failed paint caused by reportError's notice repaint; the | 432 | + | // that nested notice repaint; leave the guard before returning so a later, |
| 433 | - | // outer failure still records the notice and attempts recovery once. | 433 | + | // independent failed transaction can still notify the user. |
| 434 | - | fallback := sess.renderCoordinator() == nil | 434 | + | if sess.renderCoordinator() == nil { |
| 435 | - | if fallback && !ac.prepareFailureFallback.CompareAndSwap(false, true) { | 435 | + | if !ac.prepareFailureFallback.CompareAndSwap(false, true) { |
| 436 | + | return true | |||
| 437 | + | } | |||
| 438 | + | d.reportError(sess, domain.UserErr(domain.NoticeInternal, "display update failed", err)) | |||
| 439 | + | ac.prepareFailureFallback.Store(false) | |||
| 436 | return true | 440 | return true | ||
| 437 | } | 441 | } | ||
| 438 | - | if fallback { | |||
| 439 | - | defer ac.prepareFailureFallback.Store(false) | |||
| 440 | - | } | |||
| 441 | d.reportError(sess, domain.UserErr(domain.NoticeInternal, "display update failed", err)) | 442 | d.reportError(sess, domain.UserErr(domain.NoticeInternal, "display update failed", err)) | ||
| 442 | d.invalidateRender(sess, ac, true, "render_pipeline.go:prepare-failed") | 443 | d.invalidateRender(sess, ac, true, "render_pipeline.go:prepare-failed") | ||
| 443 | return true | 444 | return true | ||
M internal/usecase/daemon/render_test.go +11 -0
| @@ -1145,6 +1145,17 @@ func TestPTYQueryGetsResponseWrittenBackToPTY(t *testing.T) { | |||
| 1145 | 1145 | } | |
| 1146 | 1146 | } | |
| 1147 | 1147 | ||
| 1148 | + | func TestCaptureCursorInputsHidesFocusedCursorForNoticesOverlay(t *testing.T) { | |
| 1149 | + | _, sess, _, _ := newManualSessionWithPTYs(t, nil) | |
| 1150 | + | p := sess.tabs[0].focusedPane() | |
| 1151 | + | ||
| 1152 | + | p.mu.Lock() | |
| 1153 | + | cursor := captureCursorInputsLocked(p, domain.Rect{Width: 80, Height: 23}, capturedOverlayRenderState{noticesOverlayActive: true}) | |
| 1154 | + | p.mu.Unlock() | |
| 1155 | + | ||
| 1156 | + | require.True(t, cursor.hiddenByOverlay) | |
| 1157 | + | } | |
| 1158 | + | ||
| 1148 | 1159 | func TestCursorTailVisibleHideAndMoveOnly(t *testing.T) { | |
| 1149 | 1160 | p, _ := newBlockingPTY(t) | |
| 1150 | 1161 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| @@ -1145,6 +1145,17 @@ func TestPTYQueryGetsResponseWrittenBackToPTY(t *testing.T) { | |||||
| 1145 | } | 1145 | } | ||
| 1146 | } | 1146 | } | ||
| 1147 | 1147 | ||||
| 1148 | + | func TestCaptureCursorInputsHidesFocusedCursorForNoticesOverlay(t *testing.T) { | |||
| 1149 | + | _, sess, _, _ := newManualSessionWithPTYs(t, nil) | |||
| 1150 | + | p := sess.tabs[0].focusedPane() | |||
| 1151 | + | ||||
| 1152 | + | p.mu.Lock() | |||
| 1153 | + | cursor := captureCursorInputsLocked(p, domain.Rect{Width: 80, Height: 23}, capturedOverlayRenderState{noticesOverlayActive: true}) | |||
| 1154 | + | p.mu.Unlock() | |||
| 1155 | + | ||||
| 1156 | + | require.True(t, cursor.hiddenByOverlay) | |||
| 1157 | + | } | |||
| 1158 | + | ||||
| 1148 | func TestCursorTailVisibleHideAndMoveOnly(t *testing.T) { | 1159 | func TestCursorTailVisibleHideAndMoveOnly(t *testing.T) { | ||
| 1149 | p, _ := newBlockingPTY(t) | 1160 | p, _ := newBlockingPTY(t) | ||
| 1150 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | 1161 | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | ||
M internal/usecase/daemon/snapshot.go +4 -2
| @@ -107,8 +107,10 @@ func (d *Daemon) restoreSnapshots(ctx context.Context) { | |||
| 107 | 107 | } | |
| 108 | 108 | if err := d.restoreSession(ctx, snap); err != nil { | |
| 109 | 109 | d.log.Warn("restoring session snapshot failed", "err", err, "session", snap.Name) | |
| 110 | - | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |
| 111 | - | "couldn't restore session "+snap.Name+" after restart", err) | |
| 110 | + | if ctx.Err() == nil { | |
| 111 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | |
| 112 | + | "couldn't restore session "+snap.Name+" after restart", err) | |
| 113 | + | } | |
| 112 | 114 | } | |
| 113 | 115 | } | |
| 114 | 116 | } | |
| @@ -107,8 +107,10 @@ func (d *Daemon) restoreSnapshots(ctx context.Context) { | |||||
| 107 | } | 107 | } | ||
| 108 | if err := d.restoreSession(ctx, snap); err != nil { | 108 | if err := d.restoreSession(ctx, snap); err != nil { | ||
| 109 | d.log.Warn("restoring session snapshot failed", "err", err, "session", snap.Name) | 109 | d.log.Warn("restoring session snapshot failed", "err", err, "session", snap.Name) | ||
| 110 | - | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, | 110 | + | if ctx.Err() == nil { |
| 111 | - | "couldn't restore session "+snap.Name+" after restart", err) | 111 | + | d.NotifyGlobal(domain.NoticeError, domain.NoticeSnapshotRestore, |
| 112 | + | "couldn't restore session "+snap.Name+" after restart", err) | |||
| 113 | + | } | |||
| 112 | } | 114 | } | ||
| 113 | } | 115 | } | ||
| 114 | } | 116 | } | ||
M internal/usecase/daemon/snapshot_restore_test.go +31 -1
| @@ -551,6 +551,31 @@ func TestRestoreSnapshotsNotifiesGlobalOnUnrestorableSnapshots(t *testing.T) { | |||
| 551 | 551 | require.Empty(t, d.notices.drainPending(), "firstPaint must consume the queue") | |
| 552 | 552 | } | |
| 553 | 553 | ||
| 554 | + | func TestRestoreSnapshotsDoesNotNotifyForCancelledRestore(t *testing.T) { | |
| 555 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 556 | + | factory := &restorePTYFactory{onOpen: cancel} | |
| 557 | + | store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "cancelled", Data: mustSnapshotBytes(t, snapcodec.Session{ | |
| 558 | + | Name: "cancelled", | |
| 559 | + | Tabs: []snapcodec.Tab{{ | |
| 560 | + | Cols: 80, Rows: 24, | |
| 561 | + | Tree: &layout.Tree{Focus: "one", Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{ | |
| 562 | + | layout.NewLeaf("one"), layout.NewLeaf("two"), | |
| 563 | + | }}}, | |
| 564 | + | Panes: []snapcodec.Pane{ | |
| 565 | + | emptyTerminalPane(t, "one", "/one"), | |
| 566 | + | emptyTerminalPane(t, "two", "/two"), | |
| 567 | + | }, | |
| 568 | + | }}, | |
| 569 | + | })}}} | |
| 570 | + | d := newTestDaemon(t, factory, stubClock{}) | |
| 571 | + | WithSnapshotStore(store)(d) | |
| 572 | + | ||
| 573 | + | d.restoreSnapshots(ctx) | |
| 574 | + | ||
| 575 | + | require.Len(t, factory.snapshotOpens(), 1, "cancellation stops restore before the next pane") | |
| 576 | + | require.Empty(t, d.notices.history(), "shutdown cancellation is not a user-visible restore failure") | |
| 577 | + | } | |
| 578 | + | ||
| 554 | 579 | func TestRestoreSnapshotsNotifiesGlobalOnResumeCommandWriteFailure(t *testing.T) { | |
| 555 | 580 | proc := &snapcodec.Process{Argv: []string{"pi"}, Strategy: processStrategyPi, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc123"}} | |
| 556 | 581 | store := processRestoreTestStore(t, "agent-session", proc) | |
| @@ -694,6 +719,7 @@ type restorePTYFactory struct { | |||
| 694 | 719 | mu sync.Mutex | |
| 695 | 720 | opens []restorePTYOpen | |
| 696 | 721 | writeErr error | |
| 722 | + | onOpen func() | |
| 697 | 723 | } | |
| 698 | 724 | ||
| 699 | 725 | type restorePTYOpen struct { | |
| @@ -707,10 +733,14 @@ type restorePTYOpen struct { | |||
| 707 | 733 | ||
| 708 | 734 | func (f *restorePTYFactory) Open(ctx context.Context, command string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) { | |
| 709 | 735 | f.mu.Lock() | |
| 710 | - | defer f.mu.Unlock() | |
| 711 | 736 | pty := newRestorePTY() | |
| 712 | 737 | pty.writeErr = f.writeErr | |
| 713 | 738 | f.opens = append(f.opens, restorePTYOpen{ctx: ctx, command: command, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty}) | |
| 739 | + | onOpen := f.onOpen | |
| 740 | + | f.mu.Unlock() | |
| 741 | + | if onOpen != nil { | |
| 742 | + | onOpen() | |
| 743 | + | } | |
| 714 | 744 | return pty, nil | |
| 715 | 745 | } | |
| 716 | 746 | ||
| @@ -551,6 +551,31 @@ func TestRestoreSnapshotsNotifiesGlobalOnUnrestorableSnapshots(t *testing.T) { | |||||
| 551 | require.Empty(t, d.notices.drainPending(), "firstPaint must consume the queue") | 551 | require.Empty(t, d.notices.drainPending(), "firstPaint must consume the queue") | ||
| 552 | } | 552 | } | ||
| 553 | 553 | ||||
| 554 | + | func TestRestoreSnapshotsDoesNotNotifyForCancelledRestore(t *testing.T) { | |||
| 555 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 556 | + | factory := &restorePTYFactory{onOpen: cancel} | |||
| 557 | + | store := &restoreSnapshotStore{blobs: []ports.SnapshotBlob{{Name: "cancelled", Data: mustSnapshotBytes(t, snapcodec.Session{ | |||
| 558 | + | Name: "cancelled", | |||
| 559 | + | Tabs: []snapcodec.Tab{{ | |||
| 560 | + | Cols: 80, Rows: 24, | |||
| 561 | + | Tree: &layout.Tree{Focus: "one", Root: &layout.Node{Kind: layout.Split, Dir: layout.Horizontal, Children: []*layout.Node{ | |||
| 562 | + | layout.NewLeaf("one"), layout.NewLeaf("two"), | |||
| 563 | + | }}}, | |||
| 564 | + | Panes: []snapcodec.Pane{ | |||
| 565 | + | emptyTerminalPane(t, "one", "/one"), | |||
| 566 | + | emptyTerminalPane(t, "two", "/two"), | |||
| 567 | + | }, | |||
| 568 | + | }}, | |||
| 569 | + | })}}} | |||
| 570 | + | d := newTestDaemon(t, factory, stubClock{}) | |||
| 571 | + | WithSnapshotStore(store)(d) | |||
| 572 | + | ||||
| 573 | + | d.restoreSnapshots(ctx) | |||
| 574 | + | ||||
| 575 | + | require.Len(t, factory.snapshotOpens(), 1, "cancellation stops restore before the next pane") | |||
| 576 | + | require.Empty(t, d.notices.history(), "shutdown cancellation is not a user-visible restore failure") | |||
| 577 | + | } | |||
| 578 | + | ||||
| 554 | func TestRestoreSnapshotsNotifiesGlobalOnResumeCommandWriteFailure(t *testing.T) { | 579 | func TestRestoreSnapshotsNotifiesGlobalOnResumeCommandWriteFailure(t *testing.T) { | ||
| 555 | proc := &snapcodec.Process{Argv: []string{"pi"}, Strategy: processStrategyPi, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc123"}} | 580 | proc := &snapcodec.Process{Argv: []string{"pi"}, Strategy: processStrategyPi, Opts: snapcodec.ProcessOpts{AgentSessionID: "abc123"}} | ||
| 556 | store := processRestoreTestStore(t, "agent-session", proc) | 581 | store := processRestoreTestStore(t, "agent-session", proc) | ||
| @@ -694,6 +719,7 @@ type restorePTYFactory struct { | |||||
| 694 | mu sync.Mutex | 719 | mu sync.Mutex | ||
| 695 | opens []restorePTYOpen | 720 | opens []restorePTYOpen | ||
| 696 | writeErr error | 721 | writeErr error | ||
| 722 | + | onOpen func() | |||
| 697 | } | 723 | } | ||
| 698 | 724 | ||||
| 699 | type restorePTYOpen struct { | 725 | type restorePTYOpen struct { | ||
| @@ -707,10 +733,14 @@ type restorePTYOpen struct { | |||||
| 707 | 733 | ||||
| 708 | func (f *restorePTYFactory) Open(ctx context.Context, command string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) { | 734 | func (f *restorePTYFactory) Open(ctx context.Context, command string, _ []string, env []string, dir string, sz domain.Size) (ports.PTY, error) { | ||
| 709 | f.mu.Lock() | 735 | f.mu.Lock() | ||
| 710 | - | defer f.mu.Unlock() | |||
| 711 | pty := newRestorePTY() | 736 | pty := newRestorePTY() | ||
| 712 | pty.writeErr = f.writeErr | 737 | pty.writeErr = f.writeErr | ||
| 713 | f.opens = append(f.opens, restorePTYOpen{ctx: ctx, command: command, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty}) | 738 | f.opens = append(f.opens, restorePTYOpen{ctx: ctx, command: command, dir: dir, size: sz, env: append([]string(nil), env...), pty: pty}) | ||
| 739 | + | onOpen := f.onOpen | |||
| 740 | + | f.mu.Unlock() | |||
| 741 | + | if onOpen != nil { | |||
| 742 | + | onOpen() | |||
| 743 | + | } | |||
| 714 | return pty, nil | 744 | return pty, nil | ||
| 715 | } | 745 | } | ||
| 716 | 746 | ||||