fix(daemon): guard palette render with palette lock
- commit
- 44ecbad6d1e394619d9ed7f93b68edc18a5b03cf
- author
- brice · · 2026-07-03 10:02 UTC
- parent
- e59f877
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
6 files · +57 -6
M README.md +2 -2
| @@ -53,7 +53,7 @@ Some terminals encode Alt+Space as a non-breaking space instead of `ESC Space`; | |||
| 53 | 53 | ||
| 54 | 54 | Open the palette with Alt+Space, then type a command code or fuzzy-match its label. Use Up/Down or Ctrl-N/Ctrl-P to move through matches, Enter to run the selected command, and Esc to close the palette. | |
| 55 | 55 | ||
| 56 | - | Ephemeral sessions are marked with `*` in the status bar, for example `0*`. Renaming an ephemeral session makes it persistent and removes the marker. | |
| 56 | + | The status bar marks ephemeral sessions with `*`, for example `0*`. Ephemeral sessions are removed when their client detaches. Renaming an ephemeral session makes it a persistent named session and removes the marker. | |
| 57 | 57 | ||
| 58 | 58 | | Code | Action | | |
| 59 | 59 | |---|---| | |
| @@ -66,7 +66,7 @@ Ephemeral sessions are marked with `*` in the status bar, for example `0*`. Rena | |||
| 66 | 66 | | RNS | rename session | | |
| 67 | 67 | | DET | detach | | |
| 68 | 68 | ||
| 69 | - | Rename prompts are prefilled with the current name. Type text, use Backspace to edit, Enter to submit, or Esc/Ctrl-C to cancel. If validation fails, the prompt stays open and shows the error. | |
| 69 | + | Rename prompts are prefilled with the current session name. Type text, use Backspace to edit, Enter to submit, or Esc/Ctrl-C to cancel. Empty names and names already used by another session are rejected; the prompt stays open and shows the validation error. | |
| 70 | 70 | ||
| 71 | 71 | ## Scrollback and visual copy | |
| 72 | 72 | ||
| @@ -53,7 +53,7 @@ Some terminals encode Alt+Space as a non-breaking space instead of `ESC Space`; | |||||
| 53 | 53 | ||||
| 54 | Open the palette with Alt+Space, then type a command code or fuzzy-match its label. Use Up/Down or Ctrl-N/Ctrl-P to move through matches, Enter to run the selected command, and Esc to close the palette. | 54 | Open the palette with Alt+Space, then type a command code or fuzzy-match its label. Use Up/Down or Ctrl-N/Ctrl-P to move through matches, Enter to run the selected command, and Esc to close the palette. | ||
| 55 | 55 | ||||
| 56 | - | Ephemeral sessions are marked with `*` in the status bar, for example `0*`. Renaming an ephemeral session makes it persistent and removes the marker. | 56 | + | The status bar marks ephemeral sessions with `*`, for example `0*`. Ephemeral sessions are removed when their client detaches. Renaming an ephemeral session makes it a persistent named session and removes the marker. |
| 57 | 57 | ||||
| 58 | | Code | Action | | 58 | | Code | Action | | ||
| 59 | |---|---| | 59 | |---|---| | ||
| @@ -66,7 +66,7 @@ Ephemeral sessions are marked with `*` in the status bar, for example `0*`. Rena | |||||
| 66 | | RNS | rename session | | 66 | | RNS | rename session | | ||
| 67 | | DET | detach | | 67 | | DET | detach | | ||
| 68 | 68 | ||||
| 69 | - | Rename prompts are prefilled with the current name. Type text, use Backspace to edit, Enter to submit, or Esc/Ctrl-C to cancel. If validation fails, the prompt stays open and shows the error. | 69 | + | Rename prompts are prefilled with the current session name. Type text, use Backspace to edit, Enter to submit, or Esc/Ctrl-C to cancel. Empty names and names already used by another session are rejected; the prompt stays open and shows the validation error. |
| 70 | 70 | ||||
| 71 | ## Scrollback and visual copy | 71 | ## Scrollback and visual copy | ||
| 72 | 72 | ||||
M internal/usecase/daemon/palette_test.go +41 -0
| @@ -2,6 +2,7 @@ package daemon | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | + | "sync" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/stretchr/testify/mock" | |
| @@ -138,6 +139,46 @@ func TestPaletteUTF8PendingCompletesFilter(t *testing.T) { | |||
| 138 | 139 | require.Equal(t, "é", ac.palette.Query()) | |
| 139 | 140 | } | |
| 140 | 141 | ||
| 142 | + | func TestPaletteRenderAndInputCanRunConcurrently(t *testing.T) { | |
| 143 | + | p, release := newBlockingPTY(t) | |
| 144 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| 145 | + | defer release() | |
| 146 | + | d.enterPalette(sess, ac) | |
| 147 | + | ||
| 148 | + | drainDone := make(chan struct{}) | |
| 149 | + | drainStopped := make(chan struct{}) | |
| 150 | + | go func() { | |
| 151 | + | defer close(drainStopped) | |
| 152 | + | for { | |
| 153 | + | select { | |
| 154 | + | case <-sends: | |
| 155 | + | case <-drainDone: | |
| 156 | + | return | |
| 157 | + | } | |
| 158 | + | } | |
| 159 | + | }() | |
| 160 | + | defer func() { | |
| 161 | + | close(drainDone) | |
| 162 | + | <-drainStopped | |
| 163 | + | }() | |
| 164 | + | ||
| 165 | + | var wg sync.WaitGroup | |
| 166 | + | wg.Add(2) | |
| 167 | + | go func() { | |
| 168 | + | defer wg.Done() | |
| 169 | + | for range 200 { | |
| 170 | + | d.paint(sess, ac, true) | |
| 171 | + | } | |
| 172 | + | }() | |
| 173 | + | go func() { | |
| 174 | + | defer wg.Done() | |
| 175 | + | for range 200 { | |
| 176 | + | d.handlePaletteInput(ac, []byte("abc\x7f")) | |
| 177 | + | } | |
| 178 | + | }() | |
| 179 | + | wg.Wait() | |
| 180 | + | } | |
| 181 | + | ||
| 141 | 182 | func TestPaletteExecMethods(t *testing.T) { | |
| 142 | 183 | p1, release1 := newBlockingPTY(t) | |
| 143 | 184 | p2, release2 := newBlockingPTY(t) | |
| @@ -2,6 +2,7 @@ package daemon | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | + | "sync" | |||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| 7 | "github.com/stretchr/testify/mock" | 8 | "github.com/stretchr/testify/mock" | ||
| @@ -138,6 +139,46 @@ func TestPaletteUTF8PendingCompletesFilter(t *testing.T) { | |||||
| 138 | require.Equal(t, "é", ac.palette.Query()) | 139 | require.Equal(t, "é", ac.palette.Query()) | ||
| 139 | } | 140 | } | ||
| 140 | 141 | ||||
| 142 | + | func TestPaletteRenderAndInputCanRunConcurrently(t *testing.T) { | |||
| 143 | + | p, release := newBlockingPTY(t) | |||
| 144 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |||
| 145 | + | defer release() | |||
| 146 | + | d.enterPalette(sess, ac) | |||
| 147 | + | ||||
| 148 | + | drainDone := make(chan struct{}) | |||
| 149 | + | drainStopped := make(chan struct{}) | |||
| 150 | + | go func() { | |||
| 151 | + | defer close(drainStopped) | |||
| 152 | + | for { | |||
| 153 | + | select { | |||
| 154 | + | case <-sends: | |||
| 155 | + | case <-drainDone: | |||
| 156 | + | return | |||
| 157 | + | } | |||
| 158 | + | } | |||
| 159 | + | }() | |||
| 160 | + | defer func() { | |||
| 161 | + | close(drainDone) | |||
| 162 | + | <-drainStopped | |||
| 163 | + | }() | |||
| 164 | + | ||||
| 165 | + | var wg sync.WaitGroup | |||
| 166 | + | wg.Add(2) | |||
| 167 | + | go func() { | |||
| 168 | + | defer wg.Done() | |||
| 169 | + | for range 200 { | |||
| 170 | + | d.paint(sess, ac, true) | |||
| 171 | + | } | |||
| 172 | + | }() | |||
| 173 | + | go func() { | |||
| 174 | + | defer wg.Done() | |||
| 175 | + | for range 200 { | |||
| 176 | + | d.handlePaletteInput(ac, []byte("abc\x7f")) | |||
| 177 | + | } | |||
| 178 | + | }() | |||
| 179 | + | wg.Wait() | |||
| 180 | + | } | |||
| 181 | + | ||||
| 141 | func TestPaletteExecMethods(t *testing.T) { | 182 | func TestPaletteExecMethods(t *testing.T) { | ||
| 142 | p1, release1 := newBlockingPTY(t) | 183 | p1, release1 := newBlockingPTY(t) | ||
| 143 | p2, release2 := newBlockingPTY(t) | 184 | p2, release2 := newBlockingPTY(t) | ||
M internal/usecase/daemon/render.go +8 -2
| @@ -263,10 +263,15 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 263 | 263 | ac.paletteMu.Lock() | |
| 264 | 264 | paletteModel := ac.palette | |
| 265 | 265 | paletteActive := paletteModel != nil | |
| 266 | - | ac.paletteMu.Unlock() | |
| 266 | + | if !paletteActive { | |
| 267 | + | ac.paletteMu.Unlock() | |
| 268 | + | } | |
| 267 | 269 | ac.promptMu.Lock() | |
| 268 | 270 | promptModel := ac.prompt | |
| 269 | 271 | promptActive := promptModel != nil | |
| 272 | + | if !promptActive { | |
| 273 | + | ac.promptMu.Unlock() | |
| 274 | + | } | |
| 270 | 275 | ||
| 271 | 276 | tb.mu.Lock() | |
| 272 | 277 | if reset || copyActive || pickerActive || paletteActive || promptActive { | |
| @@ -287,11 +292,12 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 287 | 292 | } | |
| 288 | 293 | if paletteActive { | |
| 289 | 294 | frame, damage = composePaletteClientFrame(paletteModel, frame) | |
| 295 | + | ac.paletteMu.Unlock() | |
| 290 | 296 | } | |
| 291 | 297 | if promptActive { | |
| 292 | 298 | frame, damage = composePromptClientFrame(promptModel, frame) | |
| 299 | + | ac.promptMu.Unlock() | |
| 293 | 300 | } | |
| 294 | - | ac.promptMu.Unlock() | |
| 295 | 301 | desiredCursor := desiredCursorOut(tb.screen, copyActive || pickerActive || paletteActive || promptActive) | |
| 296 | 302 | data, err := ac.rend.Draw(frame, damage) | |
| 297 | 303 | var cursorTail []byte | |
| @@ -263,10 +263,15 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 263 | ac.paletteMu.Lock() | 263 | ac.paletteMu.Lock() | ||
| 264 | paletteModel := ac.palette | 264 | paletteModel := ac.palette | ||
| 265 | paletteActive := paletteModel != nil | 265 | paletteActive := paletteModel != nil | ||
| 266 | - | ac.paletteMu.Unlock() | 266 | + | if !paletteActive { |
| 267 | + | ac.paletteMu.Unlock() | |||
| 268 | + | } | |||
| 267 | ac.promptMu.Lock() | 269 | ac.promptMu.Lock() | ||
| 268 | promptModel := ac.prompt | 270 | promptModel := ac.prompt | ||
| 269 | promptActive := promptModel != nil | 271 | promptActive := promptModel != nil | ||
| 272 | + | if !promptActive { | |||
| 273 | + | ac.promptMu.Unlock() | |||
| 274 | + | } | |||
| 270 | 275 | ||||
| 271 | tb.mu.Lock() | 276 | tb.mu.Lock() | ||
| 272 | if reset || copyActive || pickerActive || paletteActive || promptActive { | 277 | if reset || copyActive || pickerActive || paletteActive || promptActive { | ||
| @@ -287,11 +292,12 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 287 | } | 292 | } | ||
| 288 | if paletteActive { | 293 | if paletteActive { | ||
| 289 | frame, damage = composePaletteClientFrame(paletteModel, frame) | 294 | frame, damage = composePaletteClientFrame(paletteModel, frame) | ||
| 295 | + | ac.paletteMu.Unlock() | |||
| 290 | } | 296 | } | ||
| 291 | if promptActive { | 297 | if promptActive { | ||
| 292 | frame, damage = composePromptClientFrame(promptModel, frame) | 298 | frame, damage = composePromptClientFrame(promptModel, frame) | ||
| 299 | + | ac.promptMu.Unlock() | |||
| 293 | } | 300 | } | ||
| 294 | - | ac.promptMu.Unlock() | |||
| 295 | desiredCursor := desiredCursorOut(tb.screen, copyActive || pickerActive || paletteActive || promptActive) | 301 | desiredCursor := desiredCursorOut(tb.screen, copyActive || pickerActive || paletteActive || promptActive) | ||
| 296 | data, err := ac.rend.Draw(frame, damage) | 302 | data, err := ac.rend.Draw(frame, damage) | ||
| 297 | var cursorTail []byte | 303 | var cursorTail []byte | ||
M internal/usecase/palette/model.go +2 -1
| @@ -103,10 +103,11 @@ func (m *Model) Render(inner domain.Size) renderer.Frame { | |||
| 103 | 103 | } | |
| 104 | 104 | base := renderer.DefaultStyle() | |
| 105 | 105 | ui.FillRect(frame, domain.Rect{Width: frame.Width, Height: frame.Height}, renderer.Cell{Rune: ' ', Style: base}) | |
| 106 | - | ui.DrawInputLine(frame, 0, "> ", m.Query(), base) | |
| 107 | 106 | if m == nil { | |
| 107 | + | ui.DrawInputLine(frame, 0, "> ", "", base) | |
| 108 | 108 | return frame | |
| 109 | 109 | } | |
| 110 | + | ui.DrawInputLine(frame, 0, "> ", m.Query(), base) | |
| 110 | 111 | visible := frame.Height - 1 | |
| 111 | 112 | if visible <= 0 { | |
| 112 | 113 | return frame | |
| @@ -103,10 +103,11 @@ func (m *Model) Render(inner domain.Size) renderer.Frame { | |||||
| 103 | } | 103 | } | ||
| 104 | base := renderer.DefaultStyle() | 104 | base := renderer.DefaultStyle() | ||
| 105 | ui.FillRect(frame, domain.Rect{Width: frame.Width, Height: frame.Height}, renderer.Cell{Rune: ' ', Style: base}) | 105 | ui.FillRect(frame, domain.Rect{Width: frame.Width, Height: frame.Height}, renderer.Cell{Rune: ' ', Style: base}) | ||
| 106 | - | ui.DrawInputLine(frame, 0, "> ", m.Query(), base) | |||
| 107 | if m == nil { | 106 | if m == nil { | ||
| 107 | + | ui.DrawInputLine(frame, 0, "> ", "", base) | |||
| 108 | return frame | 108 | return frame | ||
| 109 | } | 109 | } | ||
| 110 | + | ui.DrawInputLine(frame, 0, "> ", m.Query(), base) | |||
| 110 | visible := frame.Height - 1 | 111 | visible := frame.Height - 1 | ||
| 111 | if visible <= 0 { | 112 | if visible <= 0 { | ||
| 112 | return frame | 113 | return frame | ||
M internal/usecase/prompt/model.go +3 -1
| @@ -63,7 +63,9 @@ func (m *Model) Render(inner domain.Size) renderer.Frame { | |||
| 63 | 63 | ui.FillRect(frame, domain.Rect{Width: frame.Width, Height: frame.Height}, renderer.Cell{Rune: ' ', Style: base}) | |
| 64 | 64 | ui.DrawInputLine(frame, 0, "> ", m.Value(), base) | |
| 65 | 65 | if frame.Height > 1 && m != nil && m.errMsg != "" { | |
| 66 | - | ui.DrawText(frame, 0, 1, frame.Width, m.errMsg, base) | |
| 66 | + | errStyle := base | |
| 67 | + | errStyle.Inverse = true | |
| 68 | + | ui.DrawText(frame, 0, 1, frame.Width, m.errMsg, errStyle) | |
| 67 | 69 | } | |
| 68 | 70 | return frame | |
| 69 | 71 | } | |
| @@ -63,7 +63,9 @@ func (m *Model) Render(inner domain.Size) renderer.Frame { | |||||
| 63 | ui.FillRect(frame, domain.Rect{Width: frame.Width, Height: frame.Height}, renderer.Cell{Rune: ' ', Style: base}) | 63 | ui.FillRect(frame, domain.Rect{Width: frame.Width, Height: frame.Height}, renderer.Cell{Rune: ' ', Style: base}) | ||
| 64 | ui.DrawInputLine(frame, 0, "> ", m.Value(), base) | 64 | ui.DrawInputLine(frame, 0, "> ", m.Value(), base) | ||
| 65 | if frame.Height > 1 && m != nil && m.errMsg != "" { | 65 | if frame.Height > 1 && m != nil && m.errMsg != "" { | ||
| 66 | - | ui.DrawText(frame, 0, 1, frame.Width, m.errMsg, base) | 66 | + | errStyle := base |
| 67 | + | errStyle.Inverse = true | |||
| 68 | + | ui.DrawText(frame, 0, 1, frame.Width, m.errMsg, errStyle) | |||
| 67 | } | 69 | } | ||
| 68 | return frame | 70 | return frame | ||
| 69 | } | 71 | } | ||
M internal/usecase/prompt/model_test.go +1 -0
| @@ -34,6 +34,7 @@ func TestModelRenderDrawsInputAndError(t *testing.T) { | |||
| 34 | 34 | require.Equal(t, "> work ", promptRowText(frame.Row(0))) | |
| 35 | 35 | require.True(t, frame.At(6, 0).Style.Inverse, "caret follows prefilled value") | |
| 36 | 36 | require.Equal(t, "name already in use ", promptRowText(frame.Row(1))) | |
| 37 | + | require.True(t, frame.At(0, 1).Style.Inverse, "error line is visually distinct") | |
| 37 | 38 | } | |
| 38 | 39 | ||
| 39 | 40 | func promptRowText(row []renderer.Cell) string { | |
| @@ -34,6 +34,7 @@ func TestModelRenderDrawsInputAndError(t *testing.T) { | |||||
| 34 | require.Equal(t, "> work ", promptRowText(frame.Row(0))) | 34 | require.Equal(t, "> work ", promptRowText(frame.Row(0))) | ||
| 35 | require.True(t, frame.At(6, 0).Style.Inverse, "caret follows prefilled value") | 35 | require.True(t, frame.At(6, 0).Style.Inverse, "caret follows prefilled value") | ||
| 36 | require.Equal(t, "name already in use ", promptRowText(frame.Row(1))) | 36 | require.Equal(t, "name already in use ", promptRowText(frame.Row(1))) | ||
| 37 | + | require.True(t, frame.At(0, 1).Style.Inverse, "error line is visually distinct") | |||
| 37 | } | 38 | } | ||
| 38 | 39 | ||||
| 39 | func promptRowText(row []renderer.Cell) string { | 40 | func promptRowText(row []renderer.Cell) string { | ||