fix(daemon): preserve control mutation failures
- commit
- 2d3364c8e09d9e8882324a7a753dbd2742948e7d
- author
- brice · · 2026-07-24 05:40 UTC
- parent
- b2b8267
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +195 -28
M internal/usecase/daemon/attention_test.go +2 -2
| @@ -326,7 +326,7 @@ func TestCloseRingingTabClearsClientBar(t *testing.T) { | |||
| 326 | 326 | before := mustOutputData(t, sends) | |
| 327 | 327 | require.Contains(t, string(before), "") | |
| 328 | 328 | ||
| 329 | - | d.closeTab(sess, ringing, true) | |
| 329 | + | require.NoError(t, d.closeTab(sess, ringing, true)) | |
| 330 | 330 | after := mustOutputData(t, sends) | |
| 331 | 331 | require.NotContains(t, string(after), "") | |
| 332 | 332 | } | |
| @@ -455,7 +455,7 @@ func TestCloseRingingTabRefreshesOtherSessionBottomBar(t *testing.T) { | |||
| 455 | 455 | before := mustOutputData(t, sendsB) | |
| 456 | 456 | require.Contains(t, string(before), string(ui.AttentionGlyph)) | |
| 457 | 457 | ||
| 458 | - | d.closeTab(sessA, ringing, true) | |
| 458 | + | require.NoError(t, d.closeTab(sessA, ringing, true)) | |
| 459 | 459 | _ = mustOutputData(t, sendsA) | |
| 460 | 460 | ||
| 461 | 461 | after := mustOutputData(t, sendsB) | |
| @@ -326,7 +326,7 @@ func TestCloseRingingTabClearsClientBar(t *testing.T) { | |||||
| 326 | before := mustOutputData(t, sends) | 326 | before := mustOutputData(t, sends) | ||
| 327 | require.Contains(t, string(before), "") | 327 | require.Contains(t, string(before), "") | ||
| 328 | 328 | ||||
| 329 | - | d.closeTab(sess, ringing, true) | 329 | + | require.NoError(t, d.closeTab(sess, ringing, true)) |
| 330 | after := mustOutputData(t, sends) | 330 | after := mustOutputData(t, sends) | ||
| 331 | require.NotContains(t, string(after), "") | 331 | require.NotContains(t, string(after), "") | ||
| 332 | } | 332 | } | ||
| @@ -455,7 +455,7 @@ func TestCloseRingingTabRefreshesOtherSessionBottomBar(t *testing.T) { | |||||
| 455 | before := mustOutputData(t, sendsB) | 455 | before := mustOutputData(t, sendsB) | ||
| 456 | require.Contains(t, string(before), string(ui.AttentionGlyph)) | 456 | require.Contains(t, string(before), string(ui.AttentionGlyph)) | ||
| 457 | 457 | ||||
| 458 | - | d.closeTab(sessA, ringing, true) | 458 | + | require.NoError(t, d.closeTab(sessA, ringing, true)) |
| 459 | _ = mustOutputData(t, sendsA) | 459 | _ = mustOutputData(t, sendsA) | ||
| 460 | 460 | ||||
| 461 | after := mustOutputData(t, sendsB) | 461 | after := mustOutputData(t, sendsB) | ||
M internal/usecase/daemon/control.go +39 -4
| @@ -243,8 +243,7 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||
| 243 | 243 | case daemonActionCreateTab: | |
| 244 | 244 | return a.d.createTab(target.session, request.viewport) | |
| 245 | 245 | case daemonActionCloseTab: | |
| 246 | - | a.d.closeTab(target.session, target.tab, true) | |
| 247 | - | return nil | |
| 246 | + | return a.d.closeTab(target.session, target.tab, true) | |
| 248 | 247 | case daemonActionSplitPane: | |
| 249 | 248 | return a.d.splitPaneAt(target.session, target.tab, target.pane, request.direction) | |
| 250 | 249 | case daemonActionStackPane: | |
| @@ -255,10 +254,16 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||
| 255 | 254 | if a.d.ptys == nil { | |
| 256 | 255 | return nil | |
| 257 | 256 | } | |
| 258 | - | if target.pane == nil { | |
| 257 | + | if !a.d.hasDaemonActionPaneTarget(target) { | |
| 259 | 258 | return layout.ErrNotFound | |
| 260 | 259 | } | |
| 261 | - | return a.d.closePane(target.session, target.tab, target.pane.id, nil, true) | |
| 260 | + | if err := a.d.closePane(target.session, target.tab, target.pane.id, nil, true); err != nil { | |
| 261 | + | return err | |
| 262 | + | } | |
| 263 | + | if a.d.hasDaemonActionPaneTarget(target) { | |
| 264 | + | return errors.New("pane close did not complete") | |
| 265 | + | } | |
| 266 | + | return nil | |
| 262 | 267 | case daemonActionFocusPane: | |
| 263 | 268 | return a.d.focusDirAt(target.session, target.tab, target.pane, request.direction) | |
| 264 | 269 | case daemonActionNextTab: | |
| @@ -274,6 +279,36 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||
| 274 | 279 | } | |
| 275 | 280 | } | |
| 276 | 281 | ||
| 282 | + | // hasDaemonActionPaneTarget verifies that an already-resolved pane still | |
| 283 | + | // belongs to the live tab and session. Close-pane uses it both before mutation | |
| 284 | + | // and as a postcondition because closing a final pane delegates to close-tab. | |
| 285 | + | func (d *Daemon) hasDaemonActionPaneTarget(target daemonActionTarget) bool { | |
| 286 | + | if target.session == nil || target.tab == nil || target.pane == nil { | |
| 287 | + | return false | |
| 288 | + | } | |
| 289 | + | d.mu.Lock() | |
| 290 | + | defer d.mu.Unlock() | |
| 291 | + | if d.sessions[target.session.id] != target.session { | |
| 292 | + | return false | |
| 293 | + | } | |
| 294 | + | target.session.mu.Lock() | |
| 295 | + | defer target.session.mu.Unlock() | |
| 296 | + | foundTab := false | |
| 297 | + | for _, tb := range target.session.tabs { | |
| 298 | + | if tb == target.tab { | |
| 299 | + | foundTab = true | |
| 300 | + | break | |
| 301 | + | } | |
| 302 | + | } | |
| 303 | + | if !foundTab { | |
| 304 | + | return false | |
| 305 | + | } | |
| 306 | + | target.tab.mu.Lock() | |
| 307 | + | defer target.tab.mu.Unlock() | |
| 308 | + | return target.tab.panes[target.pane.id] == target.pane && | |
| 309 | + | target.tab.tree != nil && layout.ContainsLeaf(target.tab.tree.Root, target.pane.id) | |
| 310 | + | } | |
| 311 | + | ||
| 277 | 312 | func (a daemonActions) switchRelative(sess *session, delta int) error { | |
| 278 | 313 | if sess.switchRelative(delta) { | |
| 279 | 314 | a.d.activateTab(sess, sess.activeTab()) | |
| @@ -243,8 +243,7 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||||
| 243 | case daemonActionCreateTab: | 243 | case daemonActionCreateTab: | ||
| 244 | return a.d.createTab(target.session, request.viewport) | 244 | return a.d.createTab(target.session, request.viewport) | ||
| 245 | case daemonActionCloseTab: | 245 | case daemonActionCloseTab: | ||
| 246 | - | a.d.closeTab(target.session, target.tab, true) | 246 | + | return a.d.closeTab(target.session, target.tab, true) |
| 247 | - | return nil | |||
| 248 | case daemonActionSplitPane: | 247 | case daemonActionSplitPane: | ||
| 249 | return a.d.splitPaneAt(target.session, target.tab, target.pane, request.direction) | 248 | return a.d.splitPaneAt(target.session, target.tab, target.pane, request.direction) | ||
| 250 | case daemonActionStackPane: | 249 | case daemonActionStackPane: | ||
| @@ -255,10 +254,16 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||||
| 255 | if a.d.ptys == nil { | 254 | if a.d.ptys == nil { | ||
| 256 | return nil | 255 | return nil | ||
| 257 | } | 256 | } | ||
| 258 | - | if target.pane == nil { | 257 | + | if !a.d.hasDaemonActionPaneTarget(target) { |
| 259 | return layout.ErrNotFound | 258 | return layout.ErrNotFound | ||
| 260 | } | 259 | } | ||
| 261 | - | return a.d.closePane(target.session, target.tab, target.pane.id, nil, true) | 260 | + | if err := a.d.closePane(target.session, target.tab, target.pane.id, nil, true); err != nil { |
| 261 | + | return err | |||
| 262 | + | } | |||
| 263 | + | if a.d.hasDaemonActionPaneTarget(target) { | |||
| 264 | + | return errors.New("pane close did not complete") | |||
| 265 | + | } | |||
| 266 | + | return nil | |||
| 262 | case daemonActionFocusPane: | 267 | case daemonActionFocusPane: | ||
| 263 | return a.d.focusDirAt(target.session, target.tab, target.pane, request.direction) | 268 | return a.d.focusDirAt(target.session, target.tab, target.pane, request.direction) | ||
| 264 | case daemonActionNextTab: | 269 | case daemonActionNextTab: | ||
| @@ -274,6 +279,36 @@ func (a daemonActions) Run(request daemonActionRequest) error { | |||||
| 274 | } | 279 | } | ||
| 275 | } | 280 | } | ||
| 276 | 281 | ||||
| 282 | + | // hasDaemonActionPaneTarget verifies that an already-resolved pane still | |||
| 283 | + | // belongs to the live tab and session. Close-pane uses it both before mutation | |||
| 284 | + | // and as a postcondition because closing a final pane delegates to close-tab. | |||
| 285 | + | func (d *Daemon) hasDaemonActionPaneTarget(target daemonActionTarget) bool { | |||
| 286 | + | if target.session == nil || target.tab == nil || target.pane == nil { | |||
| 287 | + | return false | |||
| 288 | + | } | |||
| 289 | + | d.mu.Lock() | |||
| 290 | + | defer d.mu.Unlock() | |||
| 291 | + | if d.sessions[target.session.id] != target.session { | |||
| 292 | + | return false | |||
| 293 | + | } | |||
| 294 | + | target.session.mu.Lock() | |||
| 295 | + | defer target.session.mu.Unlock() | |||
| 296 | + | foundTab := false | |||
| 297 | + | for _, tb := range target.session.tabs { | |||
| 298 | + | if tb == target.tab { | |||
| 299 | + | foundTab = true | |||
| 300 | + | break | |||
| 301 | + | } | |||
| 302 | + | } | |||
| 303 | + | if !foundTab { | |||
| 304 | + | return false | |||
| 305 | + | } | |||
| 306 | + | target.tab.mu.Lock() | |||
| 307 | + | defer target.tab.mu.Unlock() | |||
| 308 | + | return target.tab.panes[target.pane.id] == target.pane && | |||
| 309 | + | target.tab.tree != nil && layout.ContainsLeaf(target.tab.tree.Root, target.pane.id) | |||
| 310 | + | } | |||
| 311 | + | ||||
| 277 | func (a daemonActions) switchRelative(sess *session, delta int) error { | 312 | func (a daemonActions) switchRelative(sess *session, delta int) error { | ||
| 278 | if sess.switchRelative(delta) { | 313 | if sess.switchRelative(delta) { | ||
| 279 | a.d.activateTab(sess, sess.activeTab()) | 314 | a.d.activateTab(sess, sess.activeTab()) | ||
M internal/usecase/daemon/control_test.go +62 -0
| @@ -9,6 +9,8 @@ import ( | |||
| 9 | 9 | ||
| 10 | 10 | "github.com/bnema/vev/internal/domain" | |
| 11 | 11 | "github.com/bnema/vev/internal/ports" | |
| 12 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |
| 13 | + | "github.com/bnema/vev/internal/usecase/command" | |
| 12 | 14 | "github.com/bnema/vev/internal/usecase/layout" | |
| 13 | 15 | "github.com/stretchr/testify/require" | |
| 14 | 16 | ) | |
| @@ -270,6 +272,66 @@ func TestHandleCommandRenameSessionRejectsInvalidNameAsCommandArgs(t *testing.T) | |||
| 270 | 272 | require.Equal(t, ports.ErrInvalidCommandArgs, result.Code) | |
| 271 | 273 | } | |
| 272 | 274 | ||
| 275 | + | func TestCloseCommandsReportMutationOutcome(t *testing.T) { | |
| 276 | + | t.Run("stale close tab target fails", func(t *testing.T) { | |
| 277 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 278 | + | sess := addControlSession(d, "work", "t_work", "p_work") | |
| 279 | + | stale := newTabWithStableID("t_stale", "p_stale", newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| 280 | + | cmd, ok := command.BySlug("close-tab") | |
| 281 | + | require.True(t, ok) | |
| 282 | + | ||
| 283 | + | result := d.runControl(cmd, controlExec{ | |
| 284 | + | d: d, sess: sess, tab: stale, | |
| 285 | + | target: daemonActionTarget{session: sess, tab: stale}, | |
| 286 | + | }, ports.CommandRequest{Slug: "close-tab"}) | |
| 287 | + | ||
| 288 | + | require.False(t, result.OK) | |
| 289 | + | require.Equal(t, ports.ErrInternal, result.Code) | |
| 290 | + | require.Len(t, sess.tabs, 1, "a stale close must retain the live tab") | |
| 291 | + | }) | |
| 292 | + | ||
| 293 | + | t.Run("failed final pane close fails and retains session", func(t *testing.T) { | |
| 294 | + | d := newTestDaemon(t, &controlPTYFactory{}, stubClock{}) | |
| 295 | + | d.snapsEnabled = true | |
| 296 | + | d.snapshotRepository = portsmocks.NewMockSnapshotRepository(t) | |
| 297 | + | sess := addControlSession(d, "work", "t_work", "p_work") | |
| 298 | + | sess.mu.Lock() | |
| 299 | + | sess.ephemeral = false | |
| 300 | + | sess.mu.Unlock() | |
| 301 | + | sess.snapEligible.Store(true) | |
| 302 | + | ||
| 303 | + | result := sendCommand(t, d, ports.CommandRequest{Slug: "close-pane", TargetSession: "work"}) | |
| 304 | + | ||
| 305 | + | require.False(t, result.OK) | |
| 306 | + | require.Equal(t, ports.ErrInternal, result.Code) | |
| 307 | + | d.mu.Lock() | |
| 308 | + | require.Same(t, sess, d.sessions[sess.id], "failed final-pane close must retain the session") | |
| 309 | + | d.mu.Unlock() | |
| 310 | + | sess.mu.Lock() | |
| 311 | + | require.Len(t, sess.tabs, 1) | |
| 312 | + | sess.mu.Unlock() | |
| 313 | + | }) | |
| 314 | + | ||
| 315 | + | t.Run("successful close tab remains successful", func(t *testing.T) { | |
| 316 | + | d := newTestDaemon(t, nil, stubClock{}) | |
| 317 | + | sess := addControlSession(d, "work", "t_first", "p_first") | |
| 318 | + | second := newTabWithStableID("t_second", "p_second", newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |
| 319 | + | second.ctx, second.cancel = context.WithCancel(d.serveCtx) | |
| 320 | + | sess.mu.Lock() | |
| 321 | + | sess.tabs = append(sess.tabs, second) | |
| 322 | + | sess.active = 1 | |
| 323 | + | sess.mu.Unlock() | |
| 324 | + | ||
| 325 | + | result := sendCommand(t, d, ports.CommandRequest{Slug: "close-tab", TargetSession: "work"}) | |
| 326 | + | ||
| 327 | + | require.True(t, result.OK, result.Text) | |
| 328 | + | sess.mu.Lock() | |
| 329 | + | require.Len(t, sess.tabs, 1) | |
| 330 | + | require.Equal(t, "t_first", sess.tabs[0].stableID) | |
| 331 | + | sess.mu.Unlock() | |
| 332 | + | }) | |
| 333 | + | } | |
| 334 | + | ||
| 273 | 335 | func TestHandleCommandHeadlessMutations(t *testing.T) { | |
| 274 | 336 | tests := []struct { | |
| 275 | 337 | name string | |
| @@ -9,6 +9,8 @@ import ( | |||||
| 9 | 9 | ||||
| 10 | "github.com/bnema/vev/internal/domain" | 10 | "github.com/bnema/vev/internal/domain" | ||
| 11 | "github.com/bnema/vev/internal/ports" | 11 | "github.com/bnema/vev/internal/ports" | ||
| 12 | + | portsmocks "github.com/bnema/vev/internal/ports/mocks" | |||
| 13 | + | "github.com/bnema/vev/internal/usecase/command" | |||
| 12 | "github.com/bnema/vev/internal/usecase/layout" | 14 | "github.com/bnema/vev/internal/usecase/layout" | ||
| 13 | "github.com/stretchr/testify/require" | 15 | "github.com/stretchr/testify/require" | ||
| 14 | ) | 16 | ) | ||
| @@ -270,6 +272,66 @@ func TestHandleCommandRenameSessionRejectsInvalidNameAsCommandArgs(t *testing.T) | |||||
| 270 | require.Equal(t, ports.ErrInvalidCommandArgs, result.Code) | 272 | require.Equal(t, ports.ErrInvalidCommandArgs, result.Code) | ||
| 271 | } | 273 | } | ||
| 272 | 274 | ||||
| 275 | + | func TestCloseCommandsReportMutationOutcome(t *testing.T) { | |||
| 276 | + | t.Run("stale close tab target fails", func(t *testing.T) { | |||
| 277 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 278 | + | sess := addControlSession(d, "work", "t_work", "p_work") | |||
| 279 | + | stale := newTabWithStableID("t_stale", "p_stale", newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |||
| 280 | + | cmd, ok := command.BySlug("close-tab") | |||
| 281 | + | require.True(t, ok) | |||
| 282 | + | ||||
| 283 | + | result := d.runControl(cmd, controlExec{ | |||
| 284 | + | d: d, sess: sess, tab: stale, | |||
| 285 | + | target: daemonActionTarget{session: sess, tab: stale}, | |||
| 286 | + | }, ports.CommandRequest{Slug: "close-tab"}) | |||
| 287 | + | ||||
| 288 | + | require.False(t, result.OK) | |||
| 289 | + | require.Equal(t, ports.ErrInternal, result.Code) | |||
| 290 | + | require.Len(t, sess.tabs, 1, "a stale close must retain the live tab") | |||
| 291 | + | }) | |||
| 292 | + | ||||
| 293 | + | t.Run("failed final pane close fails and retains session", func(t *testing.T) { | |||
| 294 | + | d := newTestDaemon(t, &controlPTYFactory{}, stubClock{}) | |||
| 295 | + | d.snapsEnabled = true | |||
| 296 | + | d.snapshotRepository = portsmocks.NewMockSnapshotRepository(t) | |||
| 297 | + | sess := addControlSession(d, "work", "t_work", "p_work") | |||
| 298 | + | sess.mu.Lock() | |||
| 299 | + | sess.ephemeral = false | |||
| 300 | + | sess.mu.Unlock() | |||
| 301 | + | sess.snapEligible.Store(true) | |||
| 302 | + | ||||
| 303 | + | result := sendCommand(t, d, ports.CommandRequest{Slug: "close-pane", TargetSession: "work"}) | |||
| 304 | + | ||||
| 305 | + | require.False(t, result.OK) | |||
| 306 | + | require.Equal(t, ports.ErrInternal, result.Code) | |||
| 307 | + | d.mu.Lock() | |||
| 308 | + | require.Same(t, sess, d.sessions[sess.id], "failed final-pane close must retain the session") | |||
| 309 | + | d.mu.Unlock() | |||
| 310 | + | sess.mu.Lock() | |||
| 311 | + | require.Len(t, sess.tabs, 1) | |||
| 312 | + | sess.mu.Unlock() | |||
| 313 | + | }) | |||
| 314 | + | ||||
| 315 | + | t.Run("successful close tab remains successful", func(t *testing.T) { | |||
| 316 | + | d := newTestDaemon(t, nil, stubClock{}) | |||
| 317 | + | sess := addControlSession(d, "work", "t_first", "p_first") | |||
| 318 | + | second := newTabWithStableID("t_second", "p_second", newQuietPTY(), domain.Size{Cols: 80, Rows: 22}) | |||
| 319 | + | second.ctx, second.cancel = context.WithCancel(d.serveCtx) | |||
| 320 | + | sess.mu.Lock() | |||
| 321 | + | sess.tabs = append(sess.tabs, second) | |||
| 322 | + | sess.active = 1 | |||
| 323 | + | sess.mu.Unlock() | |||
| 324 | + | ||||
| 325 | + | result := sendCommand(t, d, ports.CommandRequest{Slug: "close-tab", TargetSession: "work"}) | |||
| 326 | + | ||||
| 327 | + | require.True(t, result.OK, result.Text) | |||
| 328 | + | sess.mu.Lock() | |||
| 329 | + | require.Len(t, sess.tabs, 1) | |||
| 330 | + | require.Equal(t, "t_first", sess.tabs[0].stableID) | |||
| 331 | + | sess.mu.Unlock() | |||
| 332 | + | }) | |||
| 333 | + | } | |||
| 334 | + | ||||
| 273 | func TestHandleCommandHeadlessMutations(t *testing.T) { | 335 | func TestHandleCommandHeadlessMutations(t *testing.T) { | ||
| 274 | tests := []struct { | 336 | tests := []struct { | ||
| 275 | name string | 337 | name string | ||
M internal/usecase/daemon/input_test.go +1 -1
| @@ -157,7 +157,7 @@ func TestCloseTabPrunesAttachedCaptureFrames(t *testing.T) { | |||
| 157 | 157 | survivor: cachedSurvivor, | |
| 158 | 158 | } | |
| 159 | 159 | ||
| 160 | - | d.closeTab(sess, closedTab, false) | |
| 160 | + | require.NoError(t, d.closeTab(sess, closedTab, false)) | |
| 161 | 161 | require.NotContains(t, ac.captureFrames, closed, "closed tab panes must not remain strongly retained by its attachment") | |
| 162 | 162 | require.Equal(t, cachedSurvivor, ac.captureFrames[survivor], "surviving tab pane keeps its incremental capture") | |
| 163 | 163 | } | |
| @@ -157,7 +157,7 @@ func TestCloseTabPrunesAttachedCaptureFrames(t *testing.T) { | |||||
| 157 | survivor: cachedSurvivor, | 157 | survivor: cachedSurvivor, | ||
| 158 | } | 158 | } | ||
| 159 | 159 | ||||
| 160 | - | d.closeTab(sess, closedTab, false) | 160 | + | require.NoError(t, d.closeTab(sess, closedTab, false)) |
| 161 | require.NotContains(t, ac.captureFrames, closed, "closed tab panes must not remain strongly retained by its attachment") | 161 | require.NotContains(t, ac.captureFrames, closed, "closed tab panes must not remain strongly retained by its attachment") | ||
| 162 | require.Equal(t, cachedSurvivor, ac.captureFrames[survivor], "surviving tab pane keeps its incremental capture") | 162 | require.Equal(t, cachedSurvivor, ac.captureFrames[survivor], "surviving tab pane keeps its incremental capture") | ||
| 163 | } | 163 | } | ||
M internal/usecase/daemon/nil_pty_test.go +1 -1
| @@ -36,7 +36,7 @@ func TestNilPTYLifecyclePathsDoNotPanic(t *testing.T) { | |||
| 36 | 36 | }) | |
| 37 | 37 | require.NotContains(t, first.panes, layout.PaneID("pane-2")) | |
| 38 | 38 | ||
| 39 | - | require.NotPanics(t, func() { d.closeTab(sess, second, false) }) | |
| 39 | + | require.NotPanics(t, func() { require.NoError(t, d.closeTab(sess, second, false)) }) | |
| 40 | 40 | require.Len(t, sess.tabs, 1) | |
| 41 | 41 | } | |
| 42 | 42 | ||
| @@ -36,7 +36,7 @@ func TestNilPTYLifecyclePathsDoNotPanic(t *testing.T) { | |||||
| 36 | }) | 36 | }) | ||
| 37 | require.NotContains(t, first.panes, layout.PaneID("pane-2")) | 37 | require.NotContains(t, first.panes, layout.PaneID("pane-2")) | ||
| 38 | 38 | ||||
| 39 | - | require.NotPanics(t, func() { d.closeTab(sess, second, false) }) | 39 | + | require.NotPanics(t, func() { require.NoError(t, d.closeTab(sess, second, false)) }) |
| 40 | require.Len(t, sess.tabs, 1) | 40 | require.Len(t, sess.tabs, 1) | ||
| 41 | } | 41 | } | ||
| 42 | 42 | ||||
M internal/usecase/daemon/notify.go +29 -3
| @@ -115,7 +115,7 @@ func (nc *noticeCenter) queueGlobal(n domain.Notification) { | |||
| 115 | 115 | nc.mu.Lock() | |
| 116 | 116 | defer nc.mu.Unlock() | |
| 117 | 117 | for i := range nc.pending { | |
| 118 | - | if nc.pending[i].Code == n.Code && nc.pending[i].SessionID == n.SessionID { | |
| 118 | + | if sameToastNotice(nc.pending[i], n) { | |
| 119 | 119 | nc.pending[i].Count += n.Count | |
| 120 | 120 | nc.pending[i].Time = n.Time | |
| 121 | 121 | return | |
| @@ -369,10 +369,9 @@ func (d *Daemon) publishToast(ac *attachedClient, n domain.Notification) bool { | |||
| 369 | 369 | ||
| 370 | 370 | rt.noticeMu.Lock() | |
| 371 | 371 | defer rt.noticeMu.Unlock() | |
| 372 | - | if i := rt.indexOfToastLocked(n.Code, n.SessionID); i >= 0 { | |
| 372 | + | if i := rt.indexOfMatchingToastLocked(n); i >= 0 { | |
| 373 | 373 | rt.noticeToasts[i].n.Count += n.Count | |
| 374 | 374 | rt.noticeToasts[i].n.Time = n.Time | |
| 375 | - | rt.noticeToasts[i].n.Details = n.Details | |
| 376 | 375 | rt.noticeToasts[i].timer.stop() | |
| 377 | 376 | d.retainToastTimerLocked(ac, &rt.noticeToasts[i]) | |
| 378 | 377 | return true | |
| @@ -391,6 +390,33 @@ func (d *Daemon) publishToast(ac *attachedClient, n domain.Notification) bool { | |||
| 391 | 390 | return true | |
| 392 | 391 | } | |
| 393 | 392 | ||
| 393 | + | func sameToastNotice(a, b domain.Notification) bool { | |
| 394 | + | if a.Code != b.Code || a.SessionID != b.SessionID { | |
| 395 | + | return false | |
| 396 | + | } | |
| 397 | + | // NoticeUser is intentionally generic: its message and selected severity | |
| 398 | + | // are the notice identity. Typed daemon notices retain their established | |
| 399 | + | // code-and-scope coalescing behavior. | |
| 400 | + | if a.Code != domain.NoticeUser { | |
| 401 | + | return true | |
| 402 | + | } | |
| 403 | + | return a.Severity == b.Severity && | |
| 404 | + | a.Message == b.Message && | |
| 405 | + | a.Details == b.Details | |
| 406 | + | } | |
| 407 | + | ||
| 408 | + | // indexOfMatchingToastLocked finds a visible toast with the code-specific | |
| 409 | + | // coalescing identity. Count and time are occurrence metadata, not identity. | |
| 410 | + | // Callers must hold noticeMu. | |
| 411 | + | func (rt *overlayRuntime) indexOfMatchingToastLocked(n domain.Notification) int { | |
| 412 | + | for i := range rt.noticeToasts { | |
| 413 | + | if sameToastNotice(rt.noticeToasts[i].n, n) { | |
| 414 | + | return i | |
| 415 | + | } | |
| 416 | + | } | |
| 417 | + | return -1 | |
| 418 | + | } | |
| 419 | + | ||
| 394 | 420 | // indexOfToastLocked finds a visible toast with the same code and scope. | |
| 395 | 421 | // Callers must hold noticeMu. | |
| 396 | 422 | func (rt *overlayRuntime) indexOfToastLocked(code domain.NoticeCode, sid domain.SessionID) int { | |
| @@ -115,7 +115,7 @@ func (nc *noticeCenter) queueGlobal(n domain.Notification) { | |||||
| 115 | nc.mu.Lock() | 115 | nc.mu.Lock() | ||
| 116 | defer nc.mu.Unlock() | 116 | defer nc.mu.Unlock() | ||
| 117 | for i := range nc.pending { | 117 | for i := range nc.pending { | ||
| 118 | - | if nc.pending[i].Code == n.Code && nc.pending[i].SessionID == n.SessionID { | 118 | + | if sameToastNotice(nc.pending[i], n) { |
| 119 | nc.pending[i].Count += n.Count | 119 | nc.pending[i].Count += n.Count | ||
| 120 | nc.pending[i].Time = n.Time | 120 | nc.pending[i].Time = n.Time | ||
| 121 | return | 121 | return | ||
| @@ -369,10 +369,9 @@ func (d *Daemon) publishToast(ac *attachedClient, n domain.Notification) bool { | |||||
| 369 | 369 | ||||
| 370 | rt.noticeMu.Lock() | 370 | rt.noticeMu.Lock() | ||
| 371 | defer rt.noticeMu.Unlock() | 371 | defer rt.noticeMu.Unlock() | ||
| 372 | - | if i := rt.indexOfToastLocked(n.Code, n.SessionID); i >= 0 { | 372 | + | if i := rt.indexOfMatchingToastLocked(n); i >= 0 { |
| 373 | rt.noticeToasts[i].n.Count += n.Count | 373 | rt.noticeToasts[i].n.Count += n.Count | ||
| 374 | rt.noticeToasts[i].n.Time = n.Time | 374 | rt.noticeToasts[i].n.Time = n.Time | ||
| 375 | - | rt.noticeToasts[i].n.Details = n.Details | |||
| 376 | rt.noticeToasts[i].timer.stop() | 375 | rt.noticeToasts[i].timer.stop() | ||
| 377 | d.retainToastTimerLocked(ac, &rt.noticeToasts[i]) | 376 | d.retainToastTimerLocked(ac, &rt.noticeToasts[i]) | ||
| 378 | return true | 377 | return true | ||
| @@ -391,6 +390,33 @@ func (d *Daemon) publishToast(ac *attachedClient, n domain.Notification) bool { | |||||
| 391 | return true | 390 | return true | ||
| 392 | } | 391 | } | ||
| 393 | 392 | ||||
| 393 | + | func sameToastNotice(a, b domain.Notification) bool { | |||
| 394 | + | if a.Code != b.Code || a.SessionID != b.SessionID { | |||
| 395 | + | return false | |||
| 396 | + | } | |||
| 397 | + | // NoticeUser is intentionally generic: its message and selected severity | |||
| 398 | + | // are the notice identity. Typed daemon notices retain their established | |||
| 399 | + | // code-and-scope coalescing behavior. | |||
| 400 | + | if a.Code != domain.NoticeUser { | |||
| 401 | + | return true | |||
| 402 | + | } | |||
| 403 | + | return a.Severity == b.Severity && | |||
| 404 | + | a.Message == b.Message && | |||
| 405 | + | a.Details == b.Details | |||
| 406 | + | } | |||
| 407 | + | ||||
| 408 | + | // indexOfMatchingToastLocked finds a visible toast with the code-specific | |||
| 409 | + | // coalescing identity. Count and time are occurrence metadata, not identity. | |||
| 410 | + | // Callers must hold noticeMu. | |||
| 411 | + | func (rt *overlayRuntime) indexOfMatchingToastLocked(n domain.Notification) int { | |||
| 412 | + | for i := range rt.noticeToasts { | |||
| 413 | + | if sameToastNotice(rt.noticeToasts[i].n, n) { | |||
| 414 | + | return i | |||
| 415 | + | } | |||
| 416 | + | } | |||
| 417 | + | return -1 | |||
| 418 | + | } | |||
| 419 | + | ||||
| 394 | // indexOfToastLocked finds a visible toast with the same code and scope. | 420 | // indexOfToastLocked finds a visible toast with the same code and scope. | ||
| 395 | // Callers must hold noticeMu. | 421 | // Callers must hold noticeMu. | ||
| 396 | func (rt *overlayRuntime) indexOfToastLocked(code domain.NoticeCode, sid domain.SessionID) int { | 422 | func (rt *overlayRuntime) indexOfToastLocked(code domain.NoticeCode, sid domain.SessionID) int { | ||
M internal/usecase/daemon/notify_test.go +48 -7
| @@ -254,10 +254,10 @@ func TestNoticeCenterPendingQueue(t *testing.T) { | |||
| 254 | 254 | for i := 0; i < 32; i++ { | |
| 255 | 255 | nc.queueGlobal(notice(domain.NoticeCode(i), fmt.Sprintf("code %d", i), "")) | |
| 256 | 256 | } | |
| 257 | - | // The queue is now full. Re-queuing an existing code must coalesce | |
| 257 | + | // The queue is now full. Re-queuing an identical notice must coalesce | |
| 258 | 258 | // in place, not hit the overflow-drop branch. | |
| 259 | - | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0 again", "")) | |
| 260 | - | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0 again", "")) | |
| 259 | + | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0", "")) | |
| 260 | + | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0", "")) | |
| 261 | 261 | pending := nc.drainPending() | |
| 262 | 262 | if len(pending) != 32 { | |
| 263 | 263 | t.Fatalf("pending len = %d, want 32 (dedup must not grow past cap)", len(pending)) | |
| @@ -775,6 +775,30 @@ func TestNotifyGlobalSerializesReplacementWithDelivery(t *testing.T) { | |||
| 775 | 775 | sess.mu.Unlock() | |
| 776 | 776 | } | |
| 777 | 777 | ||
| 778 | + | func TestDetachedNoticeCoalescingPreservesDistinctContentAndSeverity(t *testing.T) { | |
| 779 | + | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | |
| 780 | + | sess.mu.Lock() | |
| 781 | + | sess.client = nil | |
| 782 | + | sess.mu.Unlock() | |
| 783 | + | ||
| 784 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build started", nil) | |
| 785 | + | d.notify(sess, domain.NoticeWarn, domain.NoticeUser, "build delayed", nil) | |
| 786 | + | d.notify(sess, domain.NoticeWarn, domain.NoticeUser, "build delayed", nil) | |
| 787 | + | ||
| 788 | + | sess.mu.Lock() | |
| 789 | + | sess.client = ac | |
| 790 | + | sess.mu.Unlock() | |
| 791 | + | d.drainPendingForFirstPaint(sess, ac) | |
| 792 | + | ||
| 793 | + | toasts := awaitToastCount(t, ac, 2) | |
| 794 | + | require.Equal(t, "build delayed", toasts[0].Message) | |
| 795 | + | require.Equal(t, domain.NoticeWarn, toasts[0].Severity) | |
| 796 | + | require.Equal(t, 2, toasts[0].Count, "identical detached notices coalesce") | |
| 797 | + | require.Equal(t, "build started", toasts[1].Message) | |
| 798 | + | require.Equal(t, domain.NoticeInfo, toasts[1].Severity) | |
| 799 | + | require.Equal(t, 1, toasts[1].Count) | |
| 800 | + | } | |
| 801 | + | ||
| 778 | 802 | func TestNotifyGlobalQueuesWhenUnattached(t *testing.T) { | |
| 779 | 803 | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | |
| 780 | 804 | sess.mu.Lock() | |
| @@ -873,19 +897,36 @@ func TestNotifyLogsAtSeverityLevel(t *testing.T) { | |||
| 873 | 897 | } | |
| 874 | 898 | ||
| 875 | 899 | func TestShowToastCoalesceAndTrim(t *testing.T) { | |
| 876 | - | t.Run("same code and scope coalesces", func(t *testing.T) { | |
| 877 | - | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | |
| 900 | + | t.Run("identical notices coalesce and renew their ttl", func(t *testing.T) { | |
| 901 | + | clk := newNoticeClock() | |
| 902 | + | d, sess, ac, _ := newNoticeFixture(t, clk) | |
| 878 | 903 | ||
| 879 | - | d.notify(sess, domain.NoticeError, domain.NoticePaneSpawn, "could not open pane", nil) | |
| 880 | - | d.notify(sess, domain.NoticeError, domain.NoticePaneSpawn, "could not open pane", nil) | |
| 904 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build finished", nil) | |
| 905 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build finished", nil) | |
| 881 | 906 | ||
| 882 | 907 | toasts := awaitToastCount(t, ac, 1) | |
| 883 | 908 | require.Equal(t, 2, toasts[0].Count) | |
| 909 | + | require.Equal(t, []time.Duration{4 * time.Second, 4 * time.Second}, clk.durations(), "coalescing renews the identical notice's ttl") | |
| 884 | 910 | _, overflow := visibleToasts(ac) | |
| 885 | 911 | require.Zero(t, overflow) | |
| 886 | 912 | require.Len(t, d.notices.history(), 2, "coalescing is display-only; history keeps both") | |
| 887 | 913 | }) | |
| 888 | 914 | ||
| 915 | + | t.Run("same code and scope preserve distinct message and severity", func(t *testing.T) { | |
| 916 | + | clk := newNoticeClock() | |
| 917 | + | d, sess, ac, _ := newNoticeFixture(t, clk) | |
| 918 | + | ||
| 919 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build started", nil) | |
| 920 | + | d.notify(sess, domain.NoticeWarn, domain.NoticeUser, "build delayed", nil) | |
| 921 | + | ||
| 922 | + | toasts := awaitToastCount(t, ac, 2) | |
| 923 | + | require.Equal(t, "build delayed", toasts[0].Message) | |
| 924 | + | require.Equal(t, domain.NoticeWarn, toasts[0].Severity) | |
| 925 | + | require.Equal(t, "build started", toasts[1].Message) | |
| 926 | + | require.Equal(t, domain.NoticeInfo, toasts[1].Severity) | |
| 927 | + | require.Equal(t, []time.Duration{4 * time.Second, 6 * time.Second}, clk.durations(), "each distinct notice keeps its severity ttl") | |
| 928 | + | }) | |
| 929 | + | ||
| 889 | 930 | t.Run("same code different scope does not coalesce", func(t *testing.T) { | |
| 890 | 931 | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | |
| 891 | 932 | ||
| @@ -254,10 +254,10 @@ func TestNoticeCenterPendingQueue(t *testing.T) { | |||||
| 254 | for i := 0; i < 32; i++ { | 254 | for i := 0; i < 32; i++ { | ||
| 255 | nc.queueGlobal(notice(domain.NoticeCode(i), fmt.Sprintf("code %d", i), "")) | 255 | nc.queueGlobal(notice(domain.NoticeCode(i), fmt.Sprintf("code %d", i), "")) | ||
| 256 | } | 256 | } | ||
| 257 | - | // The queue is now full. Re-queuing an existing code must coalesce | 257 | + | // The queue is now full. Re-queuing an identical notice must coalesce |
| 258 | // in place, not hit the overflow-drop branch. | 258 | // in place, not hit the overflow-drop branch. | ||
| 259 | - | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0 again", "")) | 259 | + | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0", "")) |
| 260 | - | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0 again", "")) | 260 | + | nc.queueGlobal(notice(domain.NoticeCode(0), "code 0", "")) |
| 261 | pending := nc.drainPending() | 261 | pending := nc.drainPending() | ||
| 262 | if len(pending) != 32 { | 262 | if len(pending) != 32 { | ||
| 263 | t.Fatalf("pending len = %d, want 32 (dedup must not grow past cap)", len(pending)) | 263 | t.Fatalf("pending len = %d, want 32 (dedup must not grow past cap)", len(pending)) | ||
| @@ -775,6 +775,30 @@ func TestNotifyGlobalSerializesReplacementWithDelivery(t *testing.T) { | |||||
| 775 | sess.mu.Unlock() | 775 | sess.mu.Unlock() | ||
| 776 | } | 776 | } | ||
| 777 | 777 | ||||
| 778 | + | func TestDetachedNoticeCoalescingPreservesDistinctContentAndSeverity(t *testing.T) { | |||
| 779 | + | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | |||
| 780 | + | sess.mu.Lock() | |||
| 781 | + | sess.client = nil | |||
| 782 | + | sess.mu.Unlock() | |||
| 783 | + | ||||
| 784 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build started", nil) | |||
| 785 | + | d.notify(sess, domain.NoticeWarn, domain.NoticeUser, "build delayed", nil) | |||
| 786 | + | d.notify(sess, domain.NoticeWarn, domain.NoticeUser, "build delayed", nil) | |||
| 787 | + | ||||
| 788 | + | sess.mu.Lock() | |||
| 789 | + | sess.client = ac | |||
| 790 | + | sess.mu.Unlock() | |||
| 791 | + | d.drainPendingForFirstPaint(sess, ac) | |||
| 792 | + | ||||
| 793 | + | toasts := awaitToastCount(t, ac, 2) | |||
| 794 | + | require.Equal(t, "build delayed", toasts[0].Message) | |||
| 795 | + | require.Equal(t, domain.NoticeWarn, toasts[0].Severity) | |||
| 796 | + | require.Equal(t, 2, toasts[0].Count, "identical detached notices coalesce") | |||
| 797 | + | require.Equal(t, "build started", toasts[1].Message) | |||
| 798 | + | require.Equal(t, domain.NoticeInfo, toasts[1].Severity) | |||
| 799 | + | require.Equal(t, 1, toasts[1].Count) | |||
| 800 | + | } | |||
| 801 | + | ||||
| 778 | func TestNotifyGlobalQueuesWhenUnattached(t *testing.T) { | 802 | func TestNotifyGlobalQueuesWhenUnattached(t *testing.T) { | ||
| 779 | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | 803 | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | ||
| 780 | sess.mu.Lock() | 804 | sess.mu.Lock() | ||
| @@ -873,19 +897,36 @@ func TestNotifyLogsAtSeverityLevel(t *testing.T) { | |||||
| 873 | } | 897 | } | ||
| 874 | 898 | ||||
| 875 | func TestShowToastCoalesceAndTrim(t *testing.T) { | 899 | func TestShowToastCoalesceAndTrim(t *testing.T) { | ||
| 876 | - | t.Run("same code and scope coalesces", func(t *testing.T) { | 900 | + | t.Run("identical notices coalesce and renew their ttl", func(t *testing.T) { |
| 877 | - | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | 901 | + | clk := newNoticeClock() |
| 902 | + | d, sess, ac, _ := newNoticeFixture(t, clk) | |||
| 878 | 903 | ||||
| 879 | - | d.notify(sess, domain.NoticeError, domain.NoticePaneSpawn, "could not open pane", nil) | 904 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build finished", nil) |
| 880 | - | d.notify(sess, domain.NoticeError, domain.NoticePaneSpawn, "could not open pane", nil) | 905 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build finished", nil) |
| 881 | 906 | ||||
| 882 | toasts := awaitToastCount(t, ac, 1) | 907 | toasts := awaitToastCount(t, ac, 1) | ||
| 883 | require.Equal(t, 2, toasts[0].Count) | 908 | require.Equal(t, 2, toasts[0].Count) | ||
| 909 | + | require.Equal(t, []time.Duration{4 * time.Second, 4 * time.Second}, clk.durations(), "coalescing renews the identical notice's ttl") | |||
| 884 | _, overflow := visibleToasts(ac) | 910 | _, overflow := visibleToasts(ac) | ||
| 885 | require.Zero(t, overflow) | 911 | require.Zero(t, overflow) | ||
| 886 | require.Len(t, d.notices.history(), 2, "coalescing is display-only; history keeps both") | 912 | require.Len(t, d.notices.history(), 2, "coalescing is display-only; history keeps both") | ||
| 887 | }) | 913 | }) | ||
| 888 | 914 | ||||
| 915 | + | t.Run("same code and scope preserve distinct message and severity", func(t *testing.T) { | |||
| 916 | + | clk := newNoticeClock() | |||
| 917 | + | d, sess, ac, _ := newNoticeFixture(t, clk) | |||
| 918 | + | ||||
| 919 | + | d.notify(sess, domain.NoticeInfo, domain.NoticeUser, "build started", nil) | |||
| 920 | + | d.notify(sess, domain.NoticeWarn, domain.NoticeUser, "build delayed", nil) | |||
| 921 | + | ||||
| 922 | + | toasts := awaitToastCount(t, ac, 2) | |||
| 923 | + | require.Equal(t, "build delayed", toasts[0].Message) | |||
| 924 | + | require.Equal(t, domain.NoticeWarn, toasts[0].Severity) | |||
| 925 | + | require.Equal(t, "build started", toasts[1].Message) | |||
| 926 | + | require.Equal(t, domain.NoticeInfo, toasts[1].Severity) | |||
| 927 | + | require.Equal(t, []time.Duration{4 * time.Second, 6 * time.Second}, clk.durations(), "each distinct notice keeps its severity ttl") | |||
| 928 | + | }) | |||
| 929 | + | ||||
| 889 | t.Run("same code different scope does not coalesce", func(t *testing.T) { | 930 | t.Run("same code different scope does not coalesce", func(t *testing.T) { | ||
| 890 | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | 931 | d, sess, ac, _ := newNoticeFixture(t, newNoticeClock()) | ||
| 891 | 932 | ||||
M internal/usecase/daemon/pane_actions.go +1 -2
| @@ -269,8 +269,7 @@ func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attache | |||
| 269 | 269 | if ac != nil { | |
| 270 | 270 | ac.overlays.clearCopyModeForPane(p) | |
| 271 | 271 | } | |
| 272 | - | d.closeTab(sess, tb, repaint) | |
| 273 | - | return nil | |
| 272 | + | return d.closeTab(sess, tb, repaint) | |
| 274 | 273 | } | |
| 275 | 274 | if err := tb.tree.Close(id); err != nil { | |
| 276 | 275 | tb.mu.Unlock() | |
| @@ -269,8 +269,7 @@ func (d *Daemon) closePane(sess *session, tb *tab, id layout.PaneID, ac *attache | |||||
| 269 | if ac != nil { | 269 | if ac != nil { | ||
| 270 | ac.overlays.clearCopyModeForPane(p) | 270 | ac.overlays.clearCopyModeForPane(p) | ||
| 271 | } | 271 | } | ||
| 272 | - | d.closeTab(sess, tb, repaint) | 272 | + | return d.closeTab(sess, tb, repaint) |
| 273 | - | return nil | |||
| 274 | } | 273 | } | ||
| 275 | if err := tb.tree.Close(id); err != nil { | 274 | if err := tb.tree.Close(id); err != nil { | ||
| 276 | tb.mu.Unlock() | 275 | tb.mu.Unlock() | ||
M internal/usecase/daemon/render_coordinator_test.go +1 -1
| @@ -1185,7 +1185,7 @@ func TestProducerInvalidations(t *testing.T) { | |||
| 1185 | 1185 | sess.mu.Lock() | |
| 1186 | 1186 | tb := sess.tabs[1] | |
| 1187 | 1187 | sess.mu.Unlock() | |
| 1188 | - | d.closeTab(sess, tb, true) | |
| 1188 | + | require.NoError(t, d.closeTab(sess, tb, true)) | |
| 1189 | 1189 | }, | |
| 1190 | 1190 | }, | |
| 1191 | 1191 | { | |
| @@ -1185,7 +1185,7 @@ func TestProducerInvalidations(t *testing.T) { | |||||
| 1185 | sess.mu.Lock() | 1185 | sess.mu.Lock() | ||
| 1186 | tb := sess.tabs[1] | 1186 | tb := sess.tabs[1] | ||
| 1187 | sess.mu.Unlock() | 1187 | sess.mu.Unlock() | ||
| 1188 | - | d.closeTab(sess, tb, true) | 1188 | + | require.NoError(t, d.closeTab(sess, tb, true)) |
| 1189 | }, | 1189 | }, | ||
| 1190 | }, | 1190 | }, | ||
| 1191 | { | 1191 | { | ||
M internal/usecase/daemon/session.go +9 -5
| @@ -737,11 +737,14 @@ func (s *session) persistRecordLocked(updatedAt int64) persist.Record { | |||
| 737 | 737 | return persist.Record{Name: s.name, Cwd: s.cwd, CreatedAt: createdAt, UpdatedAt: updatedAt, LastUsedSeq: s.mruAt.Load(), TabNames: tabNames} | |
| 738 | 738 | } | |
| 739 | 739 | ||
| 740 | - | func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |
| 740 | + | func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) error { | |
| 741 | + | if sess == nil || tb == nil { | |
| 742 | + | return layout.ErrNotFound | |
| 743 | + | } | |
| 741 | 744 | d.mu.Lock() | |
| 742 | 745 | if d.sessions[sess.id] != sess { | |
| 743 | 746 | d.mu.Unlock() | |
| 744 | - | return | |
| 747 | + | return errors.New("daemon: session closed") | |
| 745 | 748 | } | |
| 746 | 749 | sess.mu.Lock() | |
| 747 | 750 | idx := -1 | |
| @@ -754,7 +757,7 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||
| 754 | 757 | if idx == -1 { | |
| 755 | 758 | sess.mu.Unlock() | |
| 756 | 759 | d.mu.Unlock() | |
| 757 | - | return | |
| 760 | + | return errors.New("tab not found") | |
| 758 | 761 | } | |
| 759 | 762 | if len(sess.tabs) == 1 { | |
| 760 | 763 | name := sess.name | |
| @@ -764,10 +767,10 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||
| 764 | 767 | d.log.Warn("closing last tab failed", "session", name, "err", err) | |
| 765 | 768 | d.reportError(sess, domain.UserErr(domain.NoticeSnapshotSaturated, | |
| 766 | 769 | "couldn't close tab: session state not yet saved; try again", err)) | |
| 767 | - | return | |
| 770 | + | return err | |
| 768 | 771 | } | |
| 769 | 772 | d.log.Info("tab closed", "session", name, "last", true) | |
| 770 | - | return | |
| 773 | + | return nil | |
| 771 | 774 | } | |
| 772 | 775 | ringing := tb.attention | |
| 773 | 776 | wasActive := idx == sess.active | |
| @@ -818,6 +821,7 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||
| 818 | 821 | if ringing { | |
| 819 | 822 | d.repaintAllAttachedClients() | |
| 820 | 823 | } | |
| 824 | + | return nil | |
| 821 | 825 | } | |
| 822 | 826 | ||
| 823 | 827 | // ptyReader drains child output into the VT screen and pokes the dirty channel | |
| @@ -737,11 +737,14 @@ func (s *session) persistRecordLocked(updatedAt int64) persist.Record { | |||||
| 737 | return persist.Record{Name: s.name, Cwd: s.cwd, CreatedAt: createdAt, UpdatedAt: updatedAt, LastUsedSeq: s.mruAt.Load(), TabNames: tabNames} | 737 | return persist.Record{Name: s.name, Cwd: s.cwd, CreatedAt: createdAt, UpdatedAt: updatedAt, LastUsedSeq: s.mruAt.Load(), TabNames: tabNames} | ||
| 738 | } | 738 | } | ||
| 739 | 739 | ||||
| 740 | - | func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | 740 | + | func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) error { |
| 741 | + | if sess == nil || tb == nil { | |||
| 742 | + | return layout.ErrNotFound | |||
| 743 | + | } | |||
| 741 | d.mu.Lock() | 744 | d.mu.Lock() | ||
| 742 | if d.sessions[sess.id] != sess { | 745 | if d.sessions[sess.id] != sess { | ||
| 743 | d.mu.Unlock() | 746 | d.mu.Unlock() | ||
| 744 | - | return | 747 | + | return errors.New("daemon: session closed") |
| 745 | } | 748 | } | ||
| 746 | sess.mu.Lock() | 749 | sess.mu.Lock() | ||
| 747 | idx := -1 | 750 | idx := -1 | ||
| @@ -754,7 +757,7 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||||
| 754 | if idx == -1 { | 757 | if idx == -1 { | ||
| 755 | sess.mu.Unlock() | 758 | sess.mu.Unlock() | ||
| 756 | d.mu.Unlock() | 759 | d.mu.Unlock() | ||
| 757 | - | return | 760 | + | return errors.New("tab not found") |
| 758 | } | 761 | } | ||
| 759 | if len(sess.tabs) == 1 { | 762 | if len(sess.tabs) == 1 { | ||
| 760 | name := sess.name | 763 | name := sess.name | ||
| @@ -764,10 +767,10 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||||
| 764 | d.log.Warn("closing last tab failed", "session", name, "err", err) | 767 | d.log.Warn("closing last tab failed", "session", name, "err", err) | ||
| 765 | d.reportError(sess, domain.UserErr(domain.NoticeSnapshotSaturated, | 768 | d.reportError(sess, domain.UserErr(domain.NoticeSnapshotSaturated, | ||
| 766 | "couldn't close tab: session state not yet saved; try again", err)) | 769 | "couldn't close tab: session state not yet saved; try again", err)) | ||
| 767 | - | return | 770 | + | return err |
| 768 | } | 771 | } | ||
| 769 | d.log.Info("tab closed", "session", name, "last", true) | 772 | d.log.Info("tab closed", "session", name, "last", true) | ||
| 770 | - | return | 773 | + | return nil |
| 771 | } | 774 | } | ||
| 772 | ringing := tb.attention | 775 | ringing := tb.attention | ||
| 773 | wasActive := idx == sess.active | 776 | wasActive := idx == sess.active | ||
| @@ -818,6 +821,7 @@ func (d *Daemon) closeTab(sess *session, tb *tab, repaint bool) { | |||||
| 818 | if ringing { | 821 | if ringing { | ||
| 819 | d.repaintAllAttachedClients() | 822 | d.repaintAllAttachedClients() | ||
| 820 | } | 823 | } | ||
| 824 | + | return nil | |||
| 821 | } | 825 | } | ||
| 822 | 826 | ||||
| 823 | // ptyReader drains child output into the VT screen and pokes the dirty channel | 827 | // ptyReader drains child output into the VT screen and pokes the dirty channel | ||
M internal/usecase/daemon/session_test.go +2 -2
| @@ -954,7 +954,7 @@ func TestTabNamePersistenceTracksTabIndexShifts(t *testing.T) { | |||
| 954 | 954 | require.NoError(t, d.renameTab(sess, sess.tabs[i], name)) | |
| 955 | 955 | } | |
| 956 | 956 | ||
| 957 | - | d.closeTab(sess, sess.tabs[tt.closeIndex], false) | |
| 957 | + | require.NoError(t, d.closeTab(sess, sess.tabs[tt.closeIndex], false)) | |
| 958 | 958 | records, err := d.persist.LoadAll() | |
| 959 | 959 | require.NoError(t, err) | |
| 960 | 960 | require.Len(t, records, 1) | |
| @@ -980,7 +980,7 @@ func TestCloseActiveTabActivatesDestinationFloatingPane(t *testing.T) { | |||
| 980 | 980 | first.mu.Unlock() | |
| 981 | 981 | closeFloatingPane(stale) | |
| 982 | 982 | ||
| 983 | - | d.closeTab(sess, closing, false) | |
| 983 | + | require.NoError(t, d.closeTab(sess, closing, false)) | |
| 984 | 984 | ||
| 985 | 985 | require.Same(t, first, sess.activeTab()) | |
| 986 | 986 | requireFloatingInitialized(t, first) | |
| @@ -954,7 +954,7 @@ func TestTabNamePersistenceTracksTabIndexShifts(t *testing.T) { | |||||
| 954 | require.NoError(t, d.renameTab(sess, sess.tabs[i], name)) | 954 | require.NoError(t, d.renameTab(sess, sess.tabs[i], name)) | ||
| 955 | } | 955 | } | ||
| 956 | 956 | ||||
| 957 | - | d.closeTab(sess, sess.tabs[tt.closeIndex], false) | 957 | + | require.NoError(t, d.closeTab(sess, sess.tabs[tt.closeIndex], false)) |
| 958 | records, err := d.persist.LoadAll() | 958 | records, err := d.persist.LoadAll() | ||
| 959 | require.NoError(t, err) | 959 | require.NoError(t, err) | ||
| 960 | require.Len(t, records, 1) | 960 | require.Len(t, records, 1) | ||
| @@ -980,7 +980,7 @@ func TestCloseActiveTabActivatesDestinationFloatingPane(t *testing.T) { | |||||
| 980 | first.mu.Unlock() | 980 | first.mu.Unlock() | ||
| 981 | closeFloatingPane(stale) | 981 | closeFloatingPane(stale) | ||
| 982 | 982 | ||||
| 983 | - | d.closeTab(sess, closing, false) | 983 | + | require.NoError(t, d.closeTab(sess, closing, false)) |
| 984 | 984 | ||||
| 985 | require.Same(t, first, sess.activeTab()) | 985 | require.Same(t, first, sess.activeTab()) | ||
| 986 | requireFloatingInitialized(t, first) | 986 | requireFloatingInitialized(t, first) | ||