fix(term): reset every entered mode in the direct fallback
The fd fallback only re-enabled autowrap and left the alt screen, so a partial emission that had got as far as cursorHide or mouseEnable handed back a terminal with the cursor hidden and mouse reporting live — state that outlives the process just as surely as the alt screen does. Extract the full reset into a visualRestore constant and use it for both the ordinary restore and the fallback, so the two cannot drift. Any prefix of the enter sequence may have reached the terminal, so the reset has to cover all of it; every mode reset in it is idempotent, which is what makes replaying it safe when nothing was emitted. The test sink now forwards a prefix before failing, which covers the case the previous all-or-nothing sink could not reach: the terminal accepts "?1049h ?7l ?25l" and then stops.
- commit
- 62f5d143bf656f3c26f41ed357ace278bec38bd0
- author
- brice · · 2026-07-25 14:20 UTC
- parent
- 28ffa89
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +60 -21
M internal/adapters/term/terminal.go +17 -10
| @@ -41,6 +41,14 @@ const ( | |||
| 41 | 41 | autowrapEnable = "\x1b[?7h" | |
| 42 | 42 | ) | |
| 43 | 43 | ||
| 44 | + | // visualRestore turns off every mode EnterRaw turns on, in reverse order, and | |
| 45 | + | // hands the normal screen back. Shared by the ordinary restore and the direct | |
| 46 | + | // fallback so the two can never drift: a partial emission may have left any | |
| 47 | + | // prefix of the enter sequence in effect, so the reset has to cover all of it. | |
| 48 | + | // Every mode reset here is idempotent. | |
| 49 | + | const visualRestore = cursorShow + cursorStyleDefault + mouseDisable + | |
| 50 | + | bracketedPasteDisable + colorSchemeDisable + autowrapEnable + altScreenExit | |
| 51 | + | ||
| 44 | 52 | // bufSize is the batched writer's buffer capacity. | |
| 45 | 53 | const bufSize = 64 * 1024 | |
| 46 | 54 | ||
| @@ -131,16 +139,15 @@ func (t *Terminal) EnterRaw() (func() error, error) { | |||
| 131 | 139 | return t.restoreFn, nil | |
| 132 | 140 | } | |
| 133 | 141 | ||
| 134 | - | // resetVisualModesDirect returns the terminal to the normal screen with | |
| 135 | - | // autowrap on, writing straight to the descriptor instead of through the | |
| 136 | - | // batched writer. It is the fallback for a writer that has already failed: | |
| 137 | - | // part of a mode sequence may have reached the terminal, and leaving it on | |
| 138 | - | // the alt screen or with autowrap off would outlive the process. Both modes | |
| 139 | - | // are idempotent, so replaying them when nothing was emitted is harmless. | |
| 140 | - | // Best effort by construction — the descriptor is usually broken too. | |
| 141 | - | // Must be called with t.mu held. | |
| 142 | + | // resetVisualModesDirect writes visualRestore straight to the descriptor | |
| 143 | + | // instead of through the batched writer. It is the fallback for a writer that | |
| 144 | + | // has already failed: any prefix of the enter sequence may have reached the | |
| 145 | + | // terminal, and a hidden cursor, live mouse reporting or the alt screen would | |
| 146 | + | // outlive the process. Replaying the resets when nothing was emitted is | |
| 147 | + | // harmless. Best effort by construction — the descriptor is usually broken | |
| 148 | + | // too. Must be called with t.mu held. | |
| 142 | 149 | func (t *Terminal) resetVisualModesDirect() { | |
| 143 | - | _, _ = t.out.WriteString(autowrapEnable + altScreenExit) | |
| 150 | + | _, _ = t.out.WriteString(visualRestore) | |
| 144 | 151 | } | |
| 145 | 152 | ||
| 146 | 153 | // makeRestoreLocked returns an idempotent restore closure for the | |
| @@ -163,7 +170,7 @@ func (t *Terminal) restore() error { | |||
| 163 | 170 | t.mu.Lock() | |
| 164 | 171 | defer t.mu.Unlock() | |
| 165 | 172 | ||
| 166 | - | _, werr := t.bw.WriteString(cursorShow + cursorStyleDefault + mouseDisable + bracketedPasteDisable + colorSchemeDisable + autowrapEnable + altScreenExit) | |
| 173 | + | _, werr := t.bw.WriteString(visualRestore) | |
| 167 | 174 | ferr := t.bw.Flush() | |
| 168 | 175 | if werr != nil || ferr != nil { | |
| 169 | 176 | t.resetVisualModesDirect() | |
| @@ -41,6 +41,14 @@ const ( | |||||
| 41 | autowrapEnable = "\x1b[?7h" | 41 | autowrapEnable = "\x1b[?7h" | ||
| 42 | ) | 42 | ) | ||
| 43 | 43 | ||||
| 44 | + | // visualRestore turns off every mode EnterRaw turns on, in reverse order, and | |||
| 45 | + | // hands the normal screen back. Shared by the ordinary restore and the direct | |||
| 46 | + | // fallback so the two can never drift: a partial emission may have left any | |||
| 47 | + | // prefix of the enter sequence in effect, so the reset has to cover all of it. | |||
| 48 | + | // Every mode reset here is idempotent. | |||
| 49 | + | const visualRestore = cursorShow + cursorStyleDefault + mouseDisable + | |||
| 50 | + | bracketedPasteDisable + colorSchemeDisable + autowrapEnable + altScreenExit | |||
| 51 | + | ||||
| 44 | // bufSize is the batched writer's buffer capacity. | 52 | // bufSize is the batched writer's buffer capacity. | ||
| 45 | const bufSize = 64 * 1024 | 53 | const bufSize = 64 * 1024 | ||
| 46 | 54 | ||||
| @@ -131,16 +139,15 @@ func (t *Terminal) EnterRaw() (func() error, error) { | |||||
| 131 | return t.restoreFn, nil | 139 | return t.restoreFn, nil | ||
| 132 | } | 140 | } | ||
| 133 | 141 | ||||
| 134 | - | // resetVisualModesDirect returns the terminal to the normal screen with | 142 | + | // resetVisualModesDirect writes visualRestore straight to the descriptor |
| 135 | - | // autowrap on, writing straight to the descriptor instead of through the | 143 | + | // instead of through the batched writer. It is the fallback for a writer that |
| 136 | - | // batched writer. It is the fallback for a writer that has already failed: | 144 | + | // has already failed: any prefix of the enter sequence may have reached the |
| 137 | - | // part of a mode sequence may have reached the terminal, and leaving it on | 145 | + | // terminal, and a hidden cursor, live mouse reporting or the alt screen would |
| 138 | - | // the alt screen or with autowrap off would outlive the process. Both modes | 146 | + | // outlive the process. Replaying the resets when nothing was emitted is |
| 139 | - | // are idempotent, so replaying them when nothing was emitted is harmless. | 147 | + | // harmless. Best effort by construction — the descriptor is usually broken |
| 140 | - | // Best effort by construction — the descriptor is usually broken too. | 148 | + | // too. Must be called with t.mu held. |
| 141 | - | // Must be called with t.mu held. | |||
| 142 | func (t *Terminal) resetVisualModesDirect() { | 149 | func (t *Terminal) resetVisualModesDirect() { | ||
| 143 | - | _, _ = t.out.WriteString(autowrapEnable + altScreenExit) | 150 | + | _, _ = t.out.WriteString(visualRestore) |
| 144 | } | 151 | } | ||
| 145 | 152 | ||||
| 146 | // makeRestoreLocked returns an idempotent restore closure for the | 153 | // makeRestoreLocked returns an idempotent restore closure for the | ||
| @@ -163,7 +170,7 @@ func (t *Terminal) restore() error { | |||||
| 163 | t.mu.Lock() | 170 | t.mu.Lock() | ||
| 164 | defer t.mu.Unlock() | 171 | defer t.mu.Unlock() | ||
| 165 | 172 | ||||
| 166 | - | _, werr := t.bw.WriteString(cursorShow + cursorStyleDefault + mouseDisable + bracketedPasteDisable + colorSchemeDisable + autowrapEnable + altScreenExit) | 173 | + | _, werr := t.bw.WriteString(visualRestore) |
| 167 | ferr := t.bw.Flush() | 174 | ferr := t.bw.Flush() | ||
| 168 | if werr != nil || ferr != nil { | 175 | if werr != nil || ferr != nil { | ||
| 169 | t.resetVisualModesDirect() | 176 | t.resetVisualModesDirect() | ||
M internal/adapters/term/terminal_test.go +43 -11
| @@ -145,12 +145,28 @@ func TestTerminal_EnterRaw_DisablesAutowrapInsideAltScreen(t *testing.T) { | |||
| 145 | 145 | } | |
| 146 | 146 | } | |
| 147 | 147 | ||
| 148 | - | // failingWriter fails every write, standing in for a tty that stops | |
| 149 | - | // accepting output midway through a mode change. | |
| 150 | - | type failingWriter struct{} | |
| 148 | + | // prefixThenFailWriter forwards at most prefix bytes to sink and then reports | |
| 149 | + | // a failure, standing in for a tty that accepts part of a mode sequence and | |
| 150 | + | // then stops. A prefix of 0 fails without emitting anything. | |
| 151 | + | type prefixThenFailWriter struct { | |
| 152 | + | sink io.Writer | |
| 153 | + | prefix int | |
| 154 | + | } | |
| 151 | 155 | ||
| 152 | - | func (failingWriter) Write(p []byte) (int, error) { | |
| 153 | - | return 0, errors.New("sink failed") | |
| 156 | + | func (w *prefixThenFailWriter) Write(p []byte) (int, error) { | |
| 157 | + | if len(p) > w.prefix { | |
| 158 | + | p = p[:w.prefix] | |
| 159 | + | } | |
| 160 | + | n := 0 | |
| 161 | + | if len(p) > 0 { | |
| 162 | + | var err error | |
| 163 | + | n, err = w.sink.Write(p) | |
| 164 | + | if err != nil { | |
| 165 | + | return n, err | |
| 166 | + | } | |
| 167 | + | } | |
| 168 | + | w.prefix -= n | |
| 169 | + | return n, errors.New("sink failed after prefix") | |
| 154 | 170 | } | |
| 155 | 171 | ||
| 156 | 172 | // When the batched writer fails, part of the mode sequence may already have | |
| @@ -159,20 +175,33 @@ func (failingWriter) Write(p []byte) (int, error) { | |||
| 159 | 175 | // leaving tm.out intact isolates that fallback: the buffered path fails, the | |
| 160 | 176 | // real descriptor still records what the fallback attempted. | |
| 161 | 177 | func TestTerminal_WriteFailure_AttemptsVisualResetOnFd(t *testing.T) { | |
| 178 | + | // Every mode EnterRaw can turn on has to be turned back off, not just the | |
| 179 | + | // alt screen: a partial emission that got as far as cursorHide/mouseEnable | |
| 180 | + | // would otherwise leave the cursor hidden and mouse reporting on. | |
| 181 | + | const wantCleanup = cursorShow + cursorStyleDefault + mouseDisable + | |
| 182 | + | bracketedPasteDisable + colorSchemeDisable + autowrapEnable + altScreenExit | |
| 183 | + | const enterSequence = altScreenEnter + autowrapDisable + cursorHide + | |
| 184 | + | mouseEnable + bracketedPasteEnable + colorSchemeEnable | |
| 185 | + | ||
| 162 | 186 | for _, tc := range []struct { | |
| 163 | 187 | name string | |
| 188 | + | prefix int // bytes the sink accepts before failing | |
| 164 | 189 | poisonAfter bool // poison the writer after EnterRaw, i.e. fail during restore | |
| 165 | 190 | want string | |
| 166 | 191 | }{ | |
| 167 | 192 | { | |
| 168 | - | name: "enter fails before the session starts", | |
| 169 | - | want: autowrapEnable + altScreenExit, | |
| 193 | + | name: "enter fails before emitting anything", | |
| 194 | + | want: wantCleanup, | |
| 195 | + | }, | |
| 196 | + | { | |
| 197 | + | name: "enter fails after emitting part of the sequence", | |
| 198 | + | prefix: len(altScreenEnter + autowrapDisable + cursorHide), | |
| 199 | + | want: altScreenEnter + autowrapDisable + cursorHide + wantCleanup, | |
| 170 | 200 | }, | |
| 171 | 201 | { | |
| 172 | 202 | name: "restore fails after a successful enter", | |
| 173 | 203 | poisonAfter: true, | |
| 174 | - | want: altScreenEnter + autowrapDisable + cursorHide + mouseEnable + | |
| 175 | - | bracketedPasteEnable + colorSchemeEnable + autowrapEnable + altScreenExit, | |
| 204 | + | want: enterSequence + wantCleanup, | |
| 176 | 205 | }, | |
| 177 | 206 | } { | |
| 178 | 207 | t.Run(tc.name, func(t *testing.T) { | |
| @@ -187,9 +216,12 @@ func TestTerminal_WriteFailure_AttemptsVisualResetOnFd(t *testing.T) { | |||
| 187 | 216 | defer func() { _ = outR.Close() }() | |
| 188 | 217 | ||
| 189 | 218 | tm := NewWithFiles(inR, outW) | |
| 219 | + | poison := func() { | |
| 220 | + | tm.bw = newBatchWriter(&prefixThenFailWriter{sink: outW, prefix: tc.prefix}, bufSize) | |
| 221 | + | } | |
| 190 | 222 | ||
| 191 | 223 | if !tc.poisonAfter { | |
| 192 | - | tm.bw = newBatchWriter(failingWriter{}, bufSize) | |
| 224 | + | poison() | |
| 193 | 225 | if _, err := tm.EnterRaw(); err == nil { | |
| 194 | 226 | t.Fatalf("EnterRaw returned nil error on a failing sink") | |
| 195 | 227 | } | |
| @@ -198,7 +230,7 @@ func TestTerminal_WriteFailure_AttemptsVisualResetOnFd(t *testing.T) { | |||
| 198 | 230 | if err != nil { | |
| 199 | 231 | t.Fatalf("EnterRaw: %v", err) | |
| 200 | 232 | } | |
| 201 | - | tm.bw = newBatchWriter(failingWriter{}, bufSize) | |
| 233 | + | poison() | |
| 202 | 234 | if err := restore(); err == nil { | |
| 203 | 235 | t.Fatalf("restore returned nil error on a failing sink") | |
| 204 | 236 | } | |
| @@ -145,12 +145,28 @@ func TestTerminal_EnterRaw_DisablesAutowrapInsideAltScreen(t *testing.T) { | |||||
| 145 | } | 145 | } | ||
| 146 | } | 146 | } | ||
| 147 | 147 | ||||
| 148 | - | // failingWriter fails every write, standing in for a tty that stops | 148 | + | // prefixThenFailWriter forwards at most prefix bytes to sink and then reports |
| 149 | - | // accepting output midway through a mode change. | 149 | + | // a failure, standing in for a tty that accepts part of a mode sequence and |
| 150 | - | type failingWriter struct{} | 150 | + | // then stops. A prefix of 0 fails without emitting anything. |
| 151 | + | type prefixThenFailWriter struct { | |||
| 152 | + | sink io.Writer | |||
| 153 | + | prefix int | |||
| 154 | + | } | |||
| 151 | 155 | ||||
| 152 | - | func (failingWriter) Write(p []byte) (int, error) { | 156 | + | func (w *prefixThenFailWriter) Write(p []byte) (int, error) { |
| 153 | - | return 0, errors.New("sink failed") | 157 | + | if len(p) > w.prefix { |
| 158 | + | p = p[:w.prefix] | |||
| 159 | + | } | |||
| 160 | + | n := 0 | |||
| 161 | + | if len(p) > 0 { | |||
| 162 | + | var err error | |||
| 163 | + | n, err = w.sink.Write(p) | |||
| 164 | + | if err != nil { | |||
| 165 | + | return n, err | |||
| 166 | + | } | |||
| 167 | + | } | |||
| 168 | + | w.prefix -= n | |||
| 169 | + | return n, errors.New("sink failed after prefix") | |||
| 154 | } | 170 | } | ||
| 155 | 171 | ||||
| 156 | // When the batched writer fails, part of the mode sequence may already have | 172 | // When the batched writer fails, part of the mode sequence may already have | ||
| @@ -159,20 +175,33 @@ func (failingWriter) Write(p []byte) (int, error) { | |||||
| 159 | // leaving tm.out intact isolates that fallback: the buffered path fails, the | 175 | // leaving tm.out intact isolates that fallback: the buffered path fails, the | ||
| 160 | // real descriptor still records what the fallback attempted. | 176 | // real descriptor still records what the fallback attempted. | ||
| 161 | func TestTerminal_WriteFailure_AttemptsVisualResetOnFd(t *testing.T) { | 177 | func TestTerminal_WriteFailure_AttemptsVisualResetOnFd(t *testing.T) { | ||
| 178 | + | // Every mode EnterRaw can turn on has to be turned back off, not just the | |||
| 179 | + | // alt screen: a partial emission that got as far as cursorHide/mouseEnable | |||
| 180 | + | // would otherwise leave the cursor hidden and mouse reporting on. | |||
| 181 | + | const wantCleanup = cursorShow + cursorStyleDefault + mouseDisable + | |||
| 182 | + | bracketedPasteDisable + colorSchemeDisable + autowrapEnable + altScreenExit | |||
| 183 | + | const enterSequence = altScreenEnter + autowrapDisable + cursorHide + | |||
| 184 | + | mouseEnable + bracketedPasteEnable + colorSchemeEnable | |||
| 185 | + | ||||
| 162 | for _, tc := range []struct { | 186 | for _, tc := range []struct { | ||
| 163 | name string | 187 | name string | ||
| 188 | + | prefix int // bytes the sink accepts before failing | |||
| 164 | poisonAfter bool // poison the writer after EnterRaw, i.e. fail during restore | 189 | poisonAfter bool // poison the writer after EnterRaw, i.e. fail during restore | ||
| 165 | want string | 190 | want string | ||
| 166 | }{ | 191 | }{ | ||
| 167 | { | 192 | { | ||
| 168 | - | name: "enter fails before the session starts", | 193 | + | name: "enter fails before emitting anything", |
| 169 | - | want: autowrapEnable + altScreenExit, | 194 | + | want: wantCleanup, |
| 195 | + | }, | |||
| 196 | + | { | |||
| 197 | + | name: "enter fails after emitting part of the sequence", | |||
| 198 | + | prefix: len(altScreenEnter + autowrapDisable + cursorHide), | |||
| 199 | + | want: altScreenEnter + autowrapDisable + cursorHide + wantCleanup, | |||
| 170 | }, | 200 | }, | ||
| 171 | { | 201 | { | ||
| 172 | name: "restore fails after a successful enter", | 202 | name: "restore fails after a successful enter", | ||
| 173 | poisonAfter: true, | 203 | poisonAfter: true, | ||
| 174 | - | want: altScreenEnter + autowrapDisable + cursorHide + mouseEnable + | 204 | + | want: enterSequence + wantCleanup, |
| 175 | - | bracketedPasteEnable + colorSchemeEnable + autowrapEnable + altScreenExit, | |||
| 176 | }, | 205 | }, | ||
| 177 | } { | 206 | } { | ||
| 178 | t.Run(tc.name, func(t *testing.T) { | 207 | t.Run(tc.name, func(t *testing.T) { | ||
| @@ -187,9 +216,12 @@ func TestTerminal_WriteFailure_AttemptsVisualResetOnFd(t *testing.T) { | |||||
| 187 | defer func() { _ = outR.Close() }() | 216 | defer func() { _ = outR.Close() }() | ||
| 188 | 217 | ||||
| 189 | tm := NewWithFiles(inR, outW) | 218 | tm := NewWithFiles(inR, outW) | ||
| 219 | + | poison := func() { | |||
| 220 | + | tm.bw = newBatchWriter(&prefixThenFailWriter{sink: outW, prefix: tc.prefix}, bufSize) | |||
| 221 | + | } | |||
| 190 | 222 | ||||
| 191 | if !tc.poisonAfter { | 223 | if !tc.poisonAfter { | ||
| 192 | - | tm.bw = newBatchWriter(failingWriter{}, bufSize) | 224 | + | poison() |
| 193 | if _, err := tm.EnterRaw(); err == nil { | 225 | if _, err := tm.EnterRaw(); err == nil { | ||
| 194 | t.Fatalf("EnterRaw returned nil error on a failing sink") | 226 | t.Fatalf("EnterRaw returned nil error on a failing sink") | ||
| 195 | } | 227 | } | ||
| @@ -198,7 +230,7 @@ func TestTerminal_WriteFailure_AttemptsVisualResetOnFd(t *testing.T) { | |||||
| 198 | if err != nil { | 230 | if err != nil { | ||
| 199 | t.Fatalf("EnterRaw: %v", err) | 231 | t.Fatalf("EnterRaw: %v", err) | ||
| 200 | } | 232 | } | ||
| 201 | - | tm.bw = newBatchWriter(failingWriter{}, bufSize) | 233 | + | poison() |
| 202 | if err := restore(); err == nil { | 234 | if err := restore(); err == nil { | ||
| 203 | t.Fatalf("restore returned nil error on a failing sink") | 235 | t.Fatalf("restore returned nil error on a failing sink") | ||
| 204 | } | 236 | } | ||